From e7f954e232bf2e9dcdd6b8262f6b5a2f989ef876 Mon Sep 17 00:00:00 2001 From: Sang Date: Thu, 24 Sep 2026 19:27:20 +0700 Subject: [PATCH 1/4] Cast in: members with the field's type, and refuse a non-list in:, at class load `in: %i[draft published]` on a :string field compared the cast String against Symbols and rejected every request, while the exported enum advertised "draft"/"published"; `in: %w[1 2 3]` on :integer did the same. List members are now cast by the field's own type at class load and stored cast, so the runtime, the JSON Schema enum and the RSpec matcher's `within` read one list. A member no cast accepts fails at boot. `in:` only had to answer include?, which let a String through as a substring test (`in: "free pro"` accepted "e"). It must now be a Range or a non-Hash Enumerable. A Range stays as written, but is refused when its endpoints cannot be compared with the field's values. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 6 ++ README.md | 9 +-- lib/permittable.rb | 107 +++++++++++++++++++++++++++++--- lib/permittable/rspec.rb | 15 +++++ spec/json_schema_spec.rb | 9 +++ spec/matchers_spec.rb | 19 ++++++ spec/permittable_spec.rb | 83 ++++++++++++++++++++++++- spec/schema_conformance_spec.rb | 16 +++++ 8 files changed, 250 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0166e30..2ffd026 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,12 @@ Contracts that don't opt in are byte-for-byte unaffected: the default format is ### Changed - **The compatibility range is now tested rather than asserted, and narrowed to what passes.** CI ran one combination — the newest of everything — while the gemspec advertised `activesupport >= 5.0, < 9`. Testing the range surfaced two real problems. On **activesupport 5.0 and 5.1 a contract cannot be declared at all**: the registry is a `class_attribute ... default: []`, and `default:` arrived in Rails 5.2, so `permit_params` died on `NoMethodError: undefined method '+' for nil`. And on **activesupport ≤ 7.0.8.4, `require "permittable"` itself raised** `NameError: uninitialized constant ActiveSupport::LoggerThreadSafeLevel::Logger`, because concurrent-ruby 1.3.5 stopped requiring `logger` for them. The floor is now **`>= 6.1`** — the oldest line the full suite is run against — and the load failure is fixed with one stdlib `require "logger"` ahead of `require "active_support"`, so the gem loads whatever the host's own boot order. +### Fixed +- **`in:` members are now cast with the field's own type, at class load.** The runtime compared the *cast* request value against the members *as authored*, so `in: %i[draft published]` on a `:string` field compared `"draft"` with `:draft` and answered `inclusion` to **every** request — while the exported schema, which stringifies Symbols, advertised `"enum": ["draft", "published"]`, the very values the server refused. `in: %w[1 2 3]` on an `:integer` field rejected every value the same way. Each member now goes through the same cast a request value does (a Symbol read as its String; `normalize:` not applied, since it rewrites what a client sent rather than what the contract says), and the contract stores the cast members — frozen, deduplicated, and still a `Set` if one was given. Request-time matching, the exported `enum` (which for `%w[1 2 3]` on an `:integer` is now `[1, 2, 3]`, not `["1", "2", "3"]`) and the RSpec matcher all read that one list; `within` casts its own argument the same way, so `within(%i[draft published])` can repeat the declaration as written. A member no cast accepts (`in: %w[1 two]` on an `:integer`) fails at class load, naming the field and the member. +- **`in:` must be a `Range` or a list of values.** The only check was `respond_to?(:include?)`, which a `String` passes — and `String#include?` is a substring test, so `in: "free pro"` accepted `"e"`, `"fr"` and `"ee p"` as plans. A `String`, a `Hash` (whose `include?` asks about keys) and anything else that is not a `Range` or an `Enumerable` list now fail at class load. +- **An `in:` `Range` the field's values cannot be compared with fails at class load.** `in: "1".."5"` on an `:integer`, or `in: 1..5` on a `:string`, made `cover?` answer `false` for every value. A Range is deliberately **not** cast — casting would change what it means (`0..Float::INFINITY` on a `:float` and `1.5..3` on an `:integer` are real bounds no cast accepts, and a `:decimal`'s `0..100` would export its `minimum` as the string `"0.0"`) — so it is kept exactly as written, and refused only when its endpoints cannot be compared with a value of the field's type, asked the way `cover?` itself asks. +The cast only **loosens** contracts that rejected every request today: no request a contract accepted before is refused now. Two declarations that used to boot now fail at class load instead — a `String` `in:` (write it as a list, `%w[free pro]`), and a list holding a member no request could ever equal, such as `nil` (an absent value never reaches `in:`; the error names `nullable: true`). One exported-docs change for lists that already worked: the `enum` now carries the field's own encoding, so `in: [1.5]` on a `:decimal` exports `"1.5"` (the same precision-safe string its `default:` already exports) and `in: [1, 2]` on a `:float` exports `1.0, 2.0`. + ## 0.8.0 (2026-09-19) diff --git a/README.md b/README.md index 749bd30..5340eb1 100644 --- a/README.md +++ b/README.md @@ -292,7 +292,7 @@ Which options are legal depends on the field kind — anything else raises at cl | Option | Scalar | Array | Nested | Meaning | |---|:---:|:---:|:---:|---| -| `in:` | ✅ | — | — | Allowed values: a `Range` (bounds-checked with `cover?`) or an `Array` | +| `in:` | ✅ | — | — | Allowed values: a `Range` (bounds-checked with `cover?`) or a list (`Array`, `Set`). List members are cast with the field's own type at class load, so `in: %i[draft published]` on a `:string` and `in: %w[1 2 3]` on an `:integer` match what a request casts to | | `format:` | ✅¹ | — | — | Regexp the value must match, or a [preset name](#format-presets): `:email`, `:uuid`, `:url`, `:slug`, `:hostname` | | `length:` | ✅¹ | ✅ | — | `Range` or `Integer`. Character count on strings, **element count** on arrays, where it short-circuits — see [the field DSL](#the-field-dsl) | | `normalize:` | ✅¹ | — | — | `:squish`, `:strip`, `:downcase`, `:upcase`, `:email`, or a Proc. Runs **first** — before the absence rule, so a value that normalizes to `""` is absent | @@ -854,7 +854,7 @@ RSpec.describe UsersController do end ``` -Chains: `for_action`, `as`, `as_array(of:)`, `required` / `optional`, `within` (`in:`), `matching` (`format:`), `with_length`, `with_default`, `virtual`, `sensitive`. Dotted paths walk nested blocks and array-of-hash blocks alike (`"line_items.sku"`). +Chains: `for_action`, `as`, `as_array(of:)`, `required` / `optional`, `within` (`in:`, cast by the field's type just as the contract's list is, so `within(%i[draft published])` repeats the declaration as written), `matching` (`format:`), `with_length`, `with_default`, `virtual`, `sensitive`. Dotted paths walk nested blocks and array-of-hash blocks alike (`"line_items.sku"`). `for_action` picks the rule exactly like a request would (`permit_rule_for`), and may be omitted only when the controller declares a single contract — an ambiguous expectation raises instead of silently checking the wrong rule. Failure messages name what the contract actually declares. @@ -938,7 +938,7 @@ Everything else the exporter cannot translate stays visible as an `x-permittable | `:string` `:integer` `:float` `:boolean` | `string` / `integer` / `number` / `boolean` | | `:date` / `:datetime` | `string` + `format: date` / `date-time` | | `:decimal` | `type: ["string", "number"]` + `format: decimal` (string is the precision-safe encoding) | -| `in:` Array / numeric Range | `enum` / `minimum` + `maximum` (exclusive ends honoured) | +| `in:` list / numeric Range | `enum` of the cast members / `minimum` + `maximum` (exclusive ends honoured) | | `length:` | `minLength`/`maxLength` on strings, `minItems`/`maxItems` on arrays | | `format:` | `pattern`, with `\A`/`\z` translated to `^`/`$` | | `default:` / `desc:` / `example:` | `default` / `description` / `examples` | @@ -1006,7 +1006,8 @@ A bad contract is a programmer error, so it fails when the class loads — never - An unknown `normalize:` or `format:` preset, listing the presets - A `format:` that is neither a `Regexp` nor a preset name - `format:`, `length:`, or `normalize:` on a non-`:string` field -- `length:` that isn't a non-negative `Integer` or a `Range`; `in:` that doesn't respond to `include?` +- `length:` that isn't a non-negative `Integer` or a `Range`; `in:` that isn't a `Range` or a list of values — a `String` is refused, since `String#include?` would match any substring (`in: "free pro"` accepted `"e"`), and so is a `Hash` +- An `in:` member the field's own type can't cast (`in: %w[1 two]` on an `:integer`), or an `in:` `Range` whose endpoints a value of the field's type can't be compared with (`in: "1".."5"` on an `:integer`) — either would reject every request as `inclusion` - A bound **no value could satisfy**: a reversed or empty `Range` (`in: 65..18`, `length: 5..2`, `length: 3...3`), an empty `in:` set, or a `length:` of 0 on a `required` field (where `""` already violates as `missing`) - `validate:` or `transform:` that isn't callable - A `default:` or `example:` that violates its own field's contract, or an array `default:`/`example:` whose elements violate `of:` — or, for an array declared with a **block**, an element that isn't a hash the block would accept diff --git a/lib/permittable.rb b/lib/permittable.rb index c314171..015ae2d 100644 --- a/lib/permittable.rb +++ b/lib/permittable.rb @@ -707,6 +707,32 @@ def absent_value?(value) value.nil? || (value.is_a?(String) && value.empty?) end + # An `in:` list as the runtime holds it: every member cast by the field's + # own type, because included_in? compares the CAST request value against + # it. Comparing against the members as authored meant `in: %i[draft + # published]` on a :string field (and `in: %w[1 2 3]` on an :integer one) + # held values no cast could ever produce, and rejected every request. + # + # A Symbol is read as its String first: it is how Ruby spells a constant + # string, and a request never carries one, so no cast accepts it as is. + # `normalize:` is deliberately not applied — it rewrites what a client + # sent, not what the contract author wrote. Duplicates the cast collapses + # ("1" and 1 on an :integer) are dropped, and a Set stays a Set, so an + # author who chose one for its O(1) include? keeps it. + # + # Returns [:ok, members] or [:error, offending_member, code], shared by + # ContractBuilder and the RSpec matcher's `within` chain so the two + # cannot read the same list differently. + def cast_in_members(type, members) + cast_members = members.map do |member| + status, value = cast(type, member.is_a?(Symbol) ? member.to_s : member) + return [:error, member, value] unless status == :ok + + value + end.uniq + [:ok, members.is_a?(Set) ? cast_members.to_set : cast_members] + end + # Range#include? walks discrete ranges; cover? is the O(1) bounds check # and the right semantics for validation. def included_in?(allowed, value) @@ -730,6 +756,13 @@ class ContractBuilder ARRAY_OPTS = %i[of length default validate virtual sensitive required transform message desc example nullable].freeze + # One value of each scalar type as a cast produces it, for asking whether + # an `in:` Range's endpoints can be compared with that type at all. + RANGE_PROBES = { + string: "", integer: 0, float: 0.0, decimal: BigDecimal("0"), boolean: true, + date: Date.new(2000, 1, 1), datetime: Time.utc(2000) + }.freeze + attr_reader :finalizer def initialize @@ -892,14 +925,7 @@ def validate_scalar_opts!(field) raise ArgumentError, "#{LABEL}: field :#{name} is required and cannot have a :default (default implies optional)" end - if field.key?(:in) - unless field[:in].respond_to?(:include?) - raise ArgumentError, "#{LABEL}: :in for field :#{name} must respond to include? (Range or Array)" - end - - assert_satisfiable!(name, :in, field[:in]) - end - + resolve_in!(field) if field.key?(:in) validate_string_only_opts!(field) validate_length!(name, field[:length]) if field.key?(:length) validate_required_length!(field) @@ -912,6 +938,71 @@ def validate_scalar_opts!(field) validate_message!(field) end + # `in:` is a Range (bounds-checked with cover?) or a list of values. It + # used to be anything answering include?, which let a String through — + # and String#include? is a SUBSTRING test, so `in: "free pro"` accepted + # "e", "fr" and "ee p". A Hash answers include? too, about its keys. + # Both are refused here, along with anything else that is not a list. + # + # A list is stored cast by the field's type (see + # Coercion.cast_in_members), so request-time matching, the exported + # enum and the RSpec matcher all read the members the runtime compares + # against. A member no request value could ever equal is a contract + # mistake, and fails here rather than as an `inclusion` on every request. + def resolve_in!(field) + name = field[:name] + allowed = field[:in] + if allowed.is_a?(Range) + assert_comparable_range!(field, allowed) + elsif allowed.is_a?(Enumerable) && !allowed.is_a?(Hash) + field[:in] = cast_in_members!(field, allowed) + else + raise ArgumentError, "#{LABEL}: :in for field :#{name} must be a Range or a list of values " \ + "such as an Array or Set (got #{allowed.inspect})" + end + assert_satisfiable!(name, :in, field[:in]) + end + + def cast_in_members!(field, allowed) + status, members, code = Coercion.cast_in_members(field[:type], allowed) + return freeze_in_members(members) if status == :ok + + # nil is the one member written on purpose, meaning "null is allowed" — + # but an absent value never reaches in:, so the fix is worth naming. + hint = members.nil? ? " — an absent value never reaches in:; declare nullable: true to accept an explicit null" : "" + raise ArgumentError, "#{LABEL}: :in for field :#{field[:name]} contains #{members.inspect}, " \ + "which is not a valid :#{field[:type]} (#{code})#{hint}" + end + + def freeze_in_members(members) + members.is_a?(Set) ? members.to_set { |member| freeze_authored(member) }.freeze : freeze_authored(members) + end + + # A Range is kept exactly as written, unlike a list: casting its + # endpoints would change what it means. `0..Float::INFINITY` on a :float + # and `1.5..3` on an :integer are real bounds whose endpoints no cast + # accepts, and a :decimal's `0..100` would become BigDecimal endpoints + # that export as the STRING "0.0" where `minimum` needs a number. + # + # What does fail every request is an endpoint the cast value cannot be + # compared with — `"1".."5"` on an :integer, `1..5` on a :string, + # `.."9.99"` on a :decimal. cover? then answers false for every value, so + # that is caught here. The probe asks exactly what cover? will — begin + # <=> value, then value <=> end — so whatever the host's own <=> allows + # (ActiveSupport lets a Date range bound a :datetime) is allowed here too. + def assert_comparable_range!(field, range) + probe = RANGE_PROBES.fetch(field[:type]) + # Wrapped in an Array so a `false` endpoint still reads as found. + stray = if !range.begin.nil? && (range.begin <=> probe).nil? then [range.begin] + elsif !range.end.nil? && (probe <=> range.end).nil? then [range.end] + end + return unless stray + + raise ArgumentError, "#{LABEL}: :in for field :#{field[:name]} is a Range of #{stray.first.class} " \ + "(#{range.inspect}), which a :#{field[:type]} value cannot be compared with — " \ + "no value could satisfy it; write the bounds as :#{field[:type]} values" + end + def validate_json_opts!(field) name = field[:name] if field[:required] && field.key?(:default) diff --git a/lib/permittable/rspec.rb b/lib/permittable/rspec.rb index acf7b56..f175ca5 100644 --- a/lib/permittable/rspec.rb +++ b/lib/permittable/rspec.rb @@ -197,6 +197,8 @@ def check_mismatch(field, key, value) when :of then "expected an array of :#{value}, but it is of: :#{field[:of]}" unless field[:of] == value when :required then required_mismatch(field, value) when :format then format_mismatch(field, value) + # Compared cast, but reported as written. + when :in then option_mismatch(field, :in, value) unless field.key?(:in) && field[:in] == cast_in(field, value) when :virtual, :sensitive, :nullable then "expected the field to be #{key}, but it is not" unless field[key] else option_mismatch(field, key, value) end @@ -227,6 +229,19 @@ def format_mismatch(field, expected) "expected format: :#{expected}, but the contract #{declared_format(field)}" end + # A contract stores an `in:` list cast by the field's type, so + # `within(%i[draft published])` — the declaration repeated as written — + # is cast the same way before comparing, by the same function. A list + # that does not cast (or a Range, kept as written by the contract too) + # is compared as given, and the failure shows both sides. + def cast_in(field, expected) + return expected unless field[:kind] == :scalar && expected.is_a?(Enumerable) && + !expected.is_a?(Range) && !expected.is_a?(Hash) + + status, members = Coercion.cast_in_members(field[:type], expected) + status == :ok ? members : expected + end + def declared_format(field) return "declares format: :#{field[:format_name]}" if field[:format_name] return "declares format: #{field[:format].inspect}" if field[:format] diff --git a/spec/json_schema_spec.rb b/spec/json_schema_spec.rb index 6fcafee..7a6864f 100644 --- a/spec/json_schema_spec.rb +++ b/spec/json_schema_spec.rb @@ -69,6 +69,15 @@ def property(name, **opts, &contract) expect(property("pct") { optional :pct, :integer, in: 0...100 }).to include("minimum" => 0, "exclusiveMaximum" => 100) end + # The enum is built from the members as the RUNTIME holds them — cast by + # the field's own type at class load — so it cannot advertise a value the + # server refuses, nor publish "1" for a field whose JSON type is integer. + it "exports the cast members, in the field's own JSON type" do + expect(property("status") { optional :status, :string, in: %i[draft published] }["enum"]).to eq(%w[draft published]) + expect(property("n") { optional :n, :integer, in: %w[1 2 3] }["enum"]).to eq([1, 2, 3]) + expect(property("day") { optional :day, :date, in: ["Sep 5, 2026"] }["enum"]).to eq(["2026-09-05"]) + end + it "carries a non-numeric Range as an extension instead of guessing" do prop = property("code") { optional :code, :string, in: "a".."m" } expect(prop["x-permittable-range"]).to eq('"a".."m"') diff --git a/spec/matchers_spec.rb b/spec/matchers_spec.rb index 7e00f23..11d60d0 100644 --- a/spec/matchers_spec.rb +++ b/spec/matchers_spec.rb @@ -65,6 +65,25 @@ def failure_of expect(message).to include("in: 1..5") end + # The contract stores in: members cast by the field's type, so the chain + # casts its own argument the same way: `within` can repeat the declaration + # as written, or name the values the runtime actually holds. + it "checks within against the cast in: members, casting its own argument the same way" do + contract = Permittable::Contract.define do + optional :status, :string, in: %i[draft published] + optional :n, :integer, in: %w[1 2 3] + end + expect(contract).to permit_param(:status).within(%i[draft published]) + expect(contract).to permit_param(:status).within(%w[draft published]) + expect(contract).to permit_param(:n).within([1, 2, 3]) + expect(contract).to permit_param(:n).within(%w[1 2 3]) + + message = failure_of { expect(contract).to permit_param(:n).within(%w[1 2]) } + expect(message).to include('expected in: ["1", "2"], but the contract declares in: [1, 2, 3]') + message = failure_of { expect(contract).to permit_param(:n).within(%w[one]) } + expect(message).to include("declares in: [1, 2, 3]") + end + it "checks required and optional" do expect(controller).to permit_param(:email).for_action(:create).required expect(controller).to permit_param(:age).for_action(:create).optional diff --git a/spec/permittable_spec.rb b/spec/permittable_spec.rb index 5dde1ff..5165587 100644 --- a/spec/permittable_spec.rb +++ b/spec/permittable_spec.rb @@ -87,9 +87,53 @@ def recording_notifications end end - it "rejects :in that does not respond to include?" do + it "rejects an :in that is neither a Range nor a list of values" do expect { permittable_class { permit_params(:create) { required :a, :integer, in: 5 } } } - .to raise_error(ArgumentError, /:in for field :a must respond to include\?/) + .to raise_error(ArgumentError, /:in for field :a must be a Range or a list of values .*\(got 5\)/) + # A Hash is Enumerable, but include? asks about its KEYS — not a list. + expect { permittable_class { permit_params(:create) { required :a, :string, in: { "x" => 1 } } } } + .to raise_error(ArgumentError, /:in for field :a must be a Range or a list of values/) + end + + # String#include? is a substring test: in: "free pro" accepted "e", "fr" + # and "ee p" as plans. + it "rejects a String :in, which would have matched any substring" do + expect { permittable_class { permit_params(:create) { optional :plan, :string, in: "free pro" } } } + .to raise_error(ArgumentError, /:in for field :plan must be a Range or a list of values .*\(got "free pro"\)/) + end + + it "rejects an :in member that the field's own type cannot cast" do + expect { permittable_class { permit_params(:create) { optional :n, :integer, in: %w[1 two] } } } + .to raise_error(ArgumentError, /:in for field :n contains "two", which is not a valid :integer \(invalid_type\)/) + expect { permittable_class { permit_params(:create) { optional :day, :date, in: ["2026-02-30"] } } } + .to raise_error(ArgumentError, /:in for field :day contains "2026-02-30", which is not a valid :date/) + expect { permittable_class { permit_params(:create) { optional :tier, :string, in: [nil, "pro"] } } } + .to raise_error(ArgumentError, /:in for field :tier contains nil.*declare nullable: true/) + end + + it "rejects an :in Range whose endpoints the field's values cannot be compared with" do + expect { permittable_class { permit_params(:create) { optional :n, :integer, in: "1".."5" } } } + .to raise_error(ArgumentError, /:in for field :n is a Range of String \("1"\.\."5"\), which a :integer value cannot be compared/) + expect { permittable_class { permit_params(:create) { optional :s, :string, in: 1..5 } } } + .to raise_error(ArgumentError, /:in for field :s is a Range of Integer/) + expect { permittable_class { permit_params(:create) { optional :price, :decimal, in: .."9.99" } } } + .to raise_error(ArgumentError, /:in for field :price is a Range of String/) + end + + it "accepts a Range whose endpoints compare with the field's values, without rewriting it" do + decl = proc do + permit_params(:create) do + optional :ratio, :float, in: 0..Float::INFINITY + optional :price, :decimal, in: 0..100 + optional :n, :integer, in: 1.5..3 + optional :day, :date, in: (Date.new(2026, 1, 1)..) + # ActiveSupport teaches Date#<=> to compare with a Time. + optional :at, :datetime, in: (Date.new(2026, 1, 1)..) + end + end + fields = permittable_class(&decl).permit_rule_for(:create)[:fields] + expect(fields.map { |f| f[:in] }) + .to eq([0..Float::INFINITY, 0..100, 1.5..3, (Date.new(2026, 1, 1)..), (Date.new(2026, 1, 1)..)]) end it "rejects a bound no value could satisfy, rather than failing every request" do @@ -497,6 +541,41 @@ def rejected(key, value, &decl) expect(e.details.first[:code]).to eq("inclusion") end + # The members used to be compared as authored against the CAST value, so + # a :string field listing Symbols rejected every request — while its + # exported enum, which stringifies Symbols, advertised the very values it + # refused. + it "casts in: members with the field's own type, so Symbols work on a :string field" do + decl = proc { permit_params(:create) { optional :status, :string, in: %i[draft published], default: "draft" } } + expect(permit({ status: "published" }, &decl)[:status]).to eq("published") + expect(permit({}, &decl)[:status]).to eq("draft") + expect(violations_for({ status: "archived" }, &decl).details).to eq([{ param: "status", code: "inclusion" }]) + end + + it "casts String in: members on an :integer field, and any listed spelling on a :date field" do + decl = proc { permit_params(:create) { optional :n, :integer, in: %w[1 2 3] } } + expect(permit({ n: "2" }, &decl)[:n]).to eq(2) + expect(permit({ n: 3 }, &decl)[:n]).to eq(3) + expect(violations_for({ n: "4" }, &decl).details).to eq([{ param: "n", code: "inclusion" }]) + + decl = proc { permit_params(:create) { optional :day, :date, in: ["2026-09-05", Date.new(2026, 9, 6)] } } + expect(permit({ day: "Sep 5, 2026" }, &decl)[:day]).to eq(Date.new(2026, 9, 5)) + expect(permit({ day: "2026-09-06" }, &decl)[:day]).to eq(Date.new(2026, 9, 6)) + end + + it "stores the cast members frozen, deduplicated, and in the container they were given in" do + decl = proc do + permit_params(:create) do + optional :n, :integer, in: ["1", 1, "01", 2] + optional :tier, :string, in: Set[:free, :pro] + end + end + n, tier = permittable_class(&decl).permit_rule_for(:create)[:fields] + expect(n[:in]).to eq([1, 2]).and be_frozen + expect(tier[:in]).to eq(Set["free", "pro"]).and be_frozen + expect(tier[:in]).to all(be_frozen) + end + it "checks format on strings" do decl = proc { permit_params(:create) { required :zip, :string, format: /\A\d{5}\z/ } } expect(permit({ zip: "12345" }, &decl)[:zip]).to eq("12345") diff --git a/spec/schema_conformance_spec.rb b/spec/schema_conformance_spec.rb index e56bc03..e1a2019 100644 --- a/spec/schema_conformance_spec.rb +++ b/spec/schema_conformance_spec.rb @@ -76,6 +76,22 @@ [{ "plan" => nil }, :null_is_absence] ] }, + "enums authored in another type than the field's" => { + # Symbols on a :string field and Strings on an :integer field. Both used + # to reject EVERY request while the exported enum advertised values + # the server refused — the exact disagreement this spec exists to catch. + contract: proc { + optional :status, :string, in: %i[draft published] + optional :n, :integer, in: %w[1 2 3] + }, + payloads: [ + [{ "status" => "draft" }, :agree], + [{ "status" => "archived" }, :agree], + [{ "n" => 2 }, :agree], + [{ "n" => 4 }, :agree], + [{ "n" => "2" }, :coerced_encoding] + ] + }, "an exclusive range" => { contract: proc { optional :pct, :integer, in: 0...100 }, payloads: [[{ "pct" => 0 }, :agree], [{ "pct" => 99 }, :agree], [{ "pct" => 100 }, :agree]] From 00fffce6014232850233adbbb07ef6962929f20b Mon Sep 17 00:00:00 2001 From: Sang Date: Fri, 25 Sep 2026 14:01:48 +0700 Subject: [PATCH 2/4] Keep every in: that worked, and publish temporal members as written Review follow-ups on the class-load in: check: - A Hash in: (the Rails enum idiom, in: Post.statuses) is read as its keys, cast like any list, so check_column_types' enum rule sees names. - An object that only answers include? is kept as given: not cast, and exported as x-permittable-custom-validation instead of an enum. - Lists, Enumerator::Lazy included, are forced to an Array at class load, so casting never runs per request. - A Time/DateTime member of a :date field is read as its date. - A nil member is dropped on a nullable field. - A String-authored :date/:datetime member is exported as written, so the enum never lists a value the server refuses. - The matcher shares the contract's list predicate (Coercion.in_list). Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 14 +++- README.md | 8 +- lib/permittable.rb | 127 +++++++++++++++++++++++--------- lib/permittable/json_schema.rb | 20 ++++- lib/permittable/rspec.rb | 15 ++-- spec/json_schema_spec.rb | 30 +++++++- spec/matchers_spec.rb | 14 ++++ spec/permittable_spec.rb | 80 ++++++++++++++++++-- spec/schema_conformance_spec.rb | 9 +++ 9 files changed, 255 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b27b70..7c2248b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,10 +61,18 @@ Contracts that don't opt in are byte-for-byte unaffected: the default format is - **Enum columns drafted as `:integer`**, rejecting the `"shipped"` every form sends. A Rails `enum` now drafts as `:string, in: Model.statuses.keys` — the model's own accessor rather than today's keys inlined, so adding a value cannot leave the contract behind — and a database default is shown as its enum key (`"pending"`, not `0`). Rails also assigns an integer-backed enum its stored integer (`status: 1` from a JSON client), which a `:string` field turns into a rejected `"1"`; the line carries a TODO saying how to admit it rather than guessing that clients send it. An enum whose name is not a method identifier (`first-status`) is reached as `Model.defined_enums["first-status"]`, so the draft still loads. - **The STI inheritance column and `lock_version` were drafted as client-writable fields.** Mass-assigning `type` changes which class the record loads as, which is a privilege-escalation shape, not a field. Drafted from columns alone, both are now omitted from the fields and named in a TODO explaining why — including where to declare `lock_version` if the app's forms round-trip it for stale-update detection, worded for the rules actually drafted. When the controller's own permit call lists one, it stays a field with a TODO instead: omitting a `lock_version` the app sends would switch stale-update detection off the day the draft is enforced. Each counts only when the model uses it — a `type` column with `self.inheritance_column = nil`, or `lock_version` with `self.lock_optimistically = false`, is an ordinary column and drafts as one. A model left with no field to draft (only `type` and `lock_version`, or nothing else with a contract type) drafts nothing, as a model with no columns does, rather than a rule that raises `a contract must declare at least one field` when pasted. - **Column names that are not symbol literals produced a draft that did not parse.** `first-name`, `2fa_enabled` and `Email Address` were emitted as `:first-name` and friends; names are now emitted with `Symbol#inspect` (`:"first-name"`), so the draft is valid Ruby whatever the schema. -- **`in:` members are now cast with the field's own type, at class load.** The runtime compared the *cast* request value against the members *as authored*, so `in: %i[draft published]` on a `:string` field compared `"draft"` with `:draft` and answered `inclusion` to **every** request — while the exported schema, which stringifies Symbols, advertised `"enum": ["draft", "published"]`, the very values the server refused. `in: %w[1 2 3]` on an `:integer` field rejected every value the same way. Each member now goes through the same cast a request value does (a Symbol read as its String; `normalize:` not applied, since it rewrites what a client sent rather than what the contract says), and the contract stores the cast members — frozen, deduplicated, and still a `Set` if one was given. Request-time matching, the exported `enum` (which for `%w[1 2 3]` on an `:integer` is now `[1, 2, 3]`, not `["1", "2", "3"]`) and the RSpec matcher all read that one list; `within` casts its own argument the same way, so `within(%i[draft published])` can repeat the declaration as written. A member no cast accepts (`in: %w[1 two]` on an `:integer`) fails at class load, naming the field and the member. -- **`in:` must be a `Range` or a list of values.** The only check was `respond_to?(:include?)`, which a `String` passes — and `String#include?` is a substring test, so `in: "free pro"` accepted `"e"`, `"fr"` and `"ee p"` as plans. A `String`, a `Hash` (whose `include?` asks about keys) and anything else that is not a `Range` or an `Enumerable` list now fail at class load. +- **`in:` members are now cast with the field's own type, at class load.** The runtime compared the *cast* request value against the members *as authored*, so `in: %i[draft published]` on a `:string` field compared `"draft"` with `:draft` and answered `inclusion` to **every** request — while the exported schema, which stringifies Symbols, advertised `"enum": ["draft", "published"]`, the very values the server refused. `in: %w[1 2 3]` on an `:integer` field rejected every value the same way. Each member now goes through the same cast a request value does (a Symbol read as its String, a `Time`/`DateTime` on a `:date` field read as its date; `normalize:` not applied, since it rewrites what a client sent rather than what the contract says), and the contract stores the cast members — frozen, deduplicated, and still a `Set` if one was given. A `Hash` is read as its keys, which is what `Hash#include?` always asked about, so the Rails enum idiom `in: Post.statuses` keeps working — and now satisfies `check_column_types`' enum rule, which reads the stored keys. Any other list, an `Enumerator::Lazy` included, is forced to an Array once at class load rather than cast per request. Request-time matching, the exported `enum` (which for `%w[1 2 3]` on an `:integer` is now `[1, 2, 3]`, not `["1", "2", "3"]`), the column guard and the RSpec matcher all read that one list; `within` reads its own argument with the same two functions, so `within(%i[draft published])` can repeat the declaration as written. A `nil` member is dropped on a `nullable:` field, where an explicit null is accepted before `in:` is consulted. +- **A `:date`/`:datetime` `in:` member written as a String is exported as written.** Re-encoding the cast `Time` printed whole seconds, so `in: ["2026-09-05T10:00:00.25Z"]` published `"2026-09-05T10:00:00Z"`, a value the server refused. The authored String went through the very cast a request does, so every published member is one the server accepts, and a spec holds each one to that. +- **`in:` can no longer be a `String`.** The only check was `respond_to?(:include?)`, which a `String` passes — and `String#include?` is a substring test, so `in: "free pro"` accepted `"e"`, `"fr"` and `"ee p"` as plans. A `String` now fails at class load, as anything answering neither `cover?` nor `include?` already did. An object of your own that answers `include?` is still used exactly as given — not cast, and exported as `x-permittable-custom-validation` rather than an `enum` it cannot list. - **An `in:` `Range` the field's values cannot be compared with fails at class load.** `in: "1".."5"` on an `:integer`, or `in: 1..5` on a `:string`, made `cover?` answer `false` for every value. A Range is deliberately **not** cast — casting would change what it means (`0..Float::INFINITY` on a `:float` and `1.5..3` on an `:integer` are real bounds no cast accepts, and a `:decimal`'s `0..100` would export its `minimum` as the string `"0.0"`) — so it is kept exactly as written, and refused only when its endpoints cannot be compared with a value of the field's type, asked the way `cover?` itself asks. -The cast only **loosens** contracts that rejected every request today: no request a contract accepted before is refused now. Two declarations that used to boot now fail at class load instead — a `String` `in:` (write it as a list, `%w[free pro]`), and a list holding a member no request could ever equal, such as `nil` (an absent value never reaches `in:`; the error names `nullable: true`). One exported-docs change for lists that already worked: the `enum` now carries the field's own encoding, so `in: [1.5]` on a `:decimal` exports `"1.5"` (the same precision-safe string its `default:` already exports) and `in: [1, 2]` on a `:float` exports `1.0, 2.0`. + +The cast only **loosens** contracts: no request a contract accepted before is refused now. Declarations that used to boot and now fail at class load, each of which answered `inclusion` to every request (or, for a `String`, let fragments through): +- a `String` `in:` — write it as a list, `in: %w[free pro]`; +- a list member the field's type cannot cast (`in: %w[1 two]` on an `:integer`), including `nil` on a field that isn't `nullable:` — the error names `nullable: true`; +- `in: [nil]` on a `nullable:` field, which lists nothing once the `nil` is dropped (an explicit null needs no `in:`); +- a `Range` whose endpoints the field's values cannot be compared with (`in: "1".."5"` on an `:integer`). + +One exported-docs change for lists that already worked: the `enum` now carries the field's own encoding, so `in: [1.5]` on a `:decimal` exports `"1.5"` (the same precision-safe string its `default:` already exports) and `in: [1, 2]` on a `:float` exports `1.0, 2.0`. ## 0.8.0 (2026-09-19) diff --git a/README.md b/README.md index e6fe183..8592dcd 100644 --- a/README.md +++ b/README.md @@ -292,7 +292,7 @@ Which options are legal depends on the field kind — anything else raises at cl | Option | Scalar | Array | Nested | Meaning | |---|:---:|:---:|:---:|---| -| `in:` | ✅ | — | — | Allowed values: a `Range` (bounds-checked with `cover?`) or a list (`Array`, `Set`). List members are cast with the field's own type at class load, so `in: %i[draft published]` on a `:string` and `in: %w[1 2 3]` on an `:integer` match what a request casts to | +| `in:` | ✅ | — | — | Allowed values: a `Range` (bounds-checked with `cover?`), a list (`Array`, `Set`, or a `Hash` read as its keys — so `in: Post.statuses` works), or your own object answering `include?` (used as given). List members are cast with the field's own type at class load, so `in: %i[draft published]` on a `:string` and `in: %w[1 2 3]` on an `:integer` match what a request casts to; a `nil` member is dropped on a `nullable:` field | | `format:` | ✅¹ | — | — | Regexp the value must match, or a [preset name](#format-presets): `:email`, `:uuid`, `:url`, `:slug`, `:hostname` | | `length:` | ✅¹ | ✅ | — | `Range` or `Integer`. Character count on strings, **element count** on arrays, where it short-circuits — see [the field DSL](#the-field-dsl) | | `normalize:` | ✅¹ | — | — | `:squish`, `:strip`, `:downcase`, `:upcase`, `:email`, or a Proc. Runs **first** — before the absence rule, so a value that normalizes to `""` is absent | @@ -1004,7 +1004,7 @@ Everything else the exporter cannot translate stays visible as an `x-permittable | `:string` `:integer` `:float` `:boolean` | `string` / `integer` / `number` / `boolean` | | `:date` / `:datetime` | `string` + `format: date` / `date-time` | | `:decimal` | `type: ["string", "number"]` + `format: decimal` (string is the precision-safe encoding) | -| `in:` list / numeric Range | `enum` of the cast members / `minimum` + `maximum` (exclusive ends honoured) | +| `in:` list / numeric Range | `enum` of the cast members (a `:date`/`:datetime` member written as a String is published as written) / `minimum` + `maximum` (exclusive ends honoured). An `in:` object that only answers `include?` is flagged `x-permittable-custom-validation` | | `length:` | `minLength`/`maxLength` on strings, `minItems`/`maxItems` on arrays | | `format:` | `pattern`, with `\A`/`\z` translated to `^`/`$` | | `default:` / `desc:` / `example:` | `default` / `description` / `examples` | @@ -1072,8 +1072,8 @@ A bad contract is a programmer error, so it fails when the class loads — never - An unknown `normalize:` or `format:` preset, listing the presets - A `format:` that is neither a `Regexp` nor a preset name - `format:`, `length:`, or `normalize:` on a non-`:string` field -- `length:` that isn't a non-negative `Integer` or a `Range`; `in:` that isn't a `Range` or a list of values — a `String` is refused, since `String#include?` would match any substring (`in: "free pro"` accepted `"e"`), and so is a `Hash` -- An `in:` member the field's own type can't cast (`in: %w[1 two]` on an `:integer`), or an `in:` `Range` whose endpoints a value of the field's type can't be compared with (`in: "1".."5"` on an `:integer`) — either would reject every request as `inclusion` +- `length:` that isn't a non-negative `Integer` or a `Range`; an `in:` that is a `String` (`String#include?` would match any substring — `in: "free pro"` accepted `"e"`), or that answers neither `cover?` nor `include?` +- An `in:` member the field's own type can't cast (`in: %w[1 two]` on an `:integer`, or `nil` on a field that isn't `nullable:`), or an `in:` `Range` whose endpoints a value of the field's type can't be compared with (`in: "1".."5"` on an `:integer`) — either would reject every request as `inclusion` - A bound **no value could satisfy**: a reversed or empty `Range` (`in: 65..18`, `length: 5..2`, `length: 3...3`), an empty `in:` set, or a `length:` of 0 on a `required` field (where `""` already violates as `missing`) - `validate:` or `transform:` that isn't callable - A `default:` or `example:` that violates its own field's contract, or an array `default:`/`example:` whose elements violate `of:` — or, for an array declared with a **block**, an element that isn't a hash the block would accept diff --git a/lib/permittable.rb b/lib/permittable.rb index 80b0141..aeb151d 100644 --- a/lib/permittable.rb +++ b/lib/permittable.rb @@ -717,24 +717,66 @@ def absent_value?(value) # published]` on a :string field (and `in: %w[1 2 3]` on an :integer one) # held values no cast could ever produce, and rejected every request. # - # A Symbol is read as its String first: it is how Ruby spells a constant - # string, and a request never carries one, so no cast accepts it as is. # `normalize:` is deliberately not applied — it rewrites what a client # sent, not what the contract author wrote. Duplicates the cast collapses # ("1" and 1 on an :integer) are dropped, and a Set stays a Set, so an - # author who chose one for its O(1) include? keeps it. + # author who chose one for its O(1) include? keeps it. A nil member is + # dropped on a nullable field, where an explicit null is accepted before + # in: is ever consulted; anywhere else it is a member no value can equal, + # and is an error like any other. # - # Returns [:ok, members] or [:error, offending_member, code], shared by - # ContractBuilder and the RSpec matcher's `within` chain so the two - # cannot read the same list differently. - def cast_in_members(type, members) - cast_members = members.map do |member| - status, value = cast(type, member.is_a?(Symbol) ? member.to_s : member) + # `members` is what in_list returned. Returns [:ok, cast, published] — + # `published` being what an exported enum lists, see published_in_member + # — or [:error, offending_member, code]. Shared by ContractBuilder and the + # RSpec matcher's `within` chain so the two cannot read a list differently. + def cast_in_members(type, members, nullable: false) + pairs = [] + members.each do |member| + next if member.nil? && nullable + + status, value = cast_in_member(type, member) return [:error, member, value] unless status == :ok - value - end.uniq - [:ok, members.is_a?(Set) ? cast_members.to_set : cast_members] + pairs << [value, published_in_member(type, member, value)] + end + pairs = pairs.uniq(&:first) + cast_members = pairs.map(&:first) + [:ok, members.is_a?(Set) ? cast_members.to_set : cast_members, pairs.map(&:last)] + end + + # The members of an `in:` that is a LIST, or nil when it is not one: a + # Range bounds rather than lists, and an object that merely answers + # include? (a host's own allowlist) is kept as given. A Hash lists its + # KEYS, which is what Hash#include? asks about — the Rails enum idiom, + # `in: Post.statuses`. Anything else Enumerable is forced to an Array + # here, once: an Enumerator::Lazy left lazy would be cast on every + # request instead of at class load. + def in_list(allowed) + return nil if allowed.is_a?(Range) || !allowed.is_a?(Enumerable) + return allowed.keys if allowed.is_a?(Hash) + + allowed.is_a?(Set) ? allowed : allowed.to_a + end + + # A Symbol is read as its String: it is how Ruby spells a constant + # string, and a request never carries one, so no cast accepts it as is. + # A Time or DateTime on a :date field is read as its date — cast_date + # would keep a DateTime whole (it IS a Date) and refuse a Time, and + # neither would ever equal the Date a request casts to. + def cast_in_member(type, member) + member = member.to_s if member.is_a?(Symbol) + member = member.to_date if type == :date && (member.is_a?(Time) || member.is_a?(DateTime)) + cast(type, member) + end + + # What an exported enum lists for one member: the cast value, re-encoded + # as JSON — except a :date/:datetime member authored as a String, which + # is published AS WRITTEN. Re-encoding a cast Time prints whole seconds, + # so "2026-09-05T10:00:00.25Z" was published as "…10:00:00Z", a value + # the server refuses. The authored String went through the very cast a + # request does, so the server accepts it by construction. + def published_in_member(type, member, value) + member.is_a?(String) && %i[date datetime].include?(type) ? member : value end # Range#include? walks discrete ranges; cover? is the O(1) bounds check @@ -942,40 +984,55 @@ def validate_scalar_opts!(field) validate_message!(field) end - # `in:` is a Range (bounds-checked with cover?) or a list of values. It - # used to be anything answering include?, which let a String through — - # and String#include? is a SUBSTRING test, so `in: "free pro"` accepted - # "e", "fr" and "ee p". A Hash answers include? too, about its keys. - # Both are refused here, along with anything else that is not a list. + # `in:` is a Range (bounds-checked with cover?), a list of values, or an + # object of the host's own that answers include? — kept exactly as given, + # since nothing here can know what it accepts. It used to be anything + # answering include?, which let a String through, and String#include? is + # a SUBSTRING test: `in: "free pro"` accepted "e", "fr" and "ee p". A + # String is refused here, along with anything answering neither. # - # A list is stored cast by the field's type (see - # Coercion.cast_in_members), so request-time matching, the exported - # enum and the RSpec matcher all read the members the runtime compares - # against. A member no request value could ever equal is a contract - # mistake, and fails here rather than as an `inclusion` on every request. + # A list (see Coercion.in_list — a Hash lists its keys) is stored cast by + # the field's type (see Coercion.cast_in_members), so request-time + # matching, the exported enum, the RSpec matcher and the column guard's + # enum rule all read the members the runtime compares against. A member + # no request value could ever equal is a contract mistake, and fails here + # rather than as an `inclusion` on every request. def resolve_in!(field) name = field[:name] allowed = field[:in] if allowed.is_a?(Range) assert_comparable_range!(field, allowed) - elsif allowed.is_a?(Enumerable) && !allowed.is_a?(Hash) - field[:in] = cast_in_members!(field, allowed) - else - raise ArgumentError, "#{LABEL}: :in for field :#{name} must be a Range or a list of values " \ - "such as an Array or Set (got #{allowed.inspect})" + elsif (members = Coercion.in_list(allowed)) + cast_in_members!(field, members) + elsif allowed.is_a?(String) || !allowed.respond_to?(:include?) + raise ArgumentError, "#{LABEL}: :in for field :#{name} must be a Range, a list of values (an Array, Set, " \ + "or a Hash read as its keys), or an object answering include? " \ + "(got #{allowed.inspect})#{string_in_hint(allowed)}" end assert_satisfiable!(name, :in, field[:in]) end - def cast_in_members!(field, allowed) - status, members, code = Coercion.cast_in_members(field[:type], allowed) - return freeze_in_members(members) if status == :ok + def string_in_hint(allowed) + return "" unless allowed.is_a?(String) + + " — String#include? would accept any substring; list the values instead, e.g. in: %w[#{allowed}]" + end + + # `published` is stored only where it differs from the cast members (a + # String-authored :date/:datetime member), so it is read as an override. + def cast_in_members!(field, members) + status, cast, published = Coercion.cast_in_members(field[:type], members, nullable: field[:nullable]) + unless status == :ok + # cast is the offending member here, and published its error code. + # nil is the one member written on purpose, meaning "null is allowed" + # — but an absent value never reaches in:, so the fix is worth naming. + hint = cast.nil? ? " — an absent value never reaches in:; declare nullable: true to accept an explicit null" : "" + raise ArgumentError, "#{LABEL}: :in for field :#{field[:name]} contains #{cast.inspect}, " \ + "which is not a valid :#{field[:type]} (#{published})#{hint}" + end - # nil is the one member written on purpose, meaning "null is allowed" — - # but an absent value never reaches in:, so the fix is worth naming. - hint = members.nil? ? " — an absent value never reaches in:; declare nullable: true to accept an explicit null" : "" - raise ArgumentError, "#{LABEL}: :in for field :#{field[:name]} contains #{members.inspect}, " \ - "which is not a valid :#{field[:type]} (#{code})#{hint}" + field[:in] = freeze_in_members(cast) + field[:in_published] = freeze_authored(published) unless published == cast.to_a end def freeze_in_members(members) diff --git a/lib/permittable/json_schema.rb b/lib/permittable/json_schema.rb index e5ad9fc..75f70d3 100644 --- a/lib/permittable/json_schema.rb +++ b/lib/permittable/json_schema.rb @@ -117,7 +117,7 @@ def nullify!(schema, field) def scalar_schema(field) schema = SCALAR_SCHEMAS.fetch(field[:type]).dup apply_format_name!(schema, field) - apply_in!(schema, field[:in]) + apply_in!(schema, field) apply_string_bounds!(schema, field) # A preset's pattern is authored by this gem rather than by the app, so # it needs no heuristic — see apply_pattern!. @@ -157,11 +157,18 @@ def array_schema(field, unknown:) schema end - def apply_in!(schema, allowed) + # A list is stored cast by the field's type, so its enum is what the + # runtime compares against; `in_published` overrides the members an + # exact re-encoding would get wrong (see Coercion.published_in_member). + # An object that only answers include? says nothing a schema can list — + # annotate flags it as custom validation instead. + def apply_in!(schema, field) + allowed = field[:in] return unless allowed + return if opaque_in?(allowed) unless allowed.is_a?(Range) - schema["enum"] = allowed.map { |v| json_value(v) } + schema["enum"] = (field[:in_published] || allowed).map { |v| json_value(v) } return end # Runtime bounds-checks Ranges with cover?; numeric endpoints map onto @@ -175,6 +182,11 @@ def apply_in!(schema, allowed) schema[allowed.exclude_end? ? "exclusiveMaximum" : "maximum"] = json_value(allowed.end) if allowed.end end + # A host's own include?-answering allowlist, kept by the contract as given. + def opaque_in?(allowed) + !allowed.nil? && !allowed.is_a?(Range) && !allowed.is_a?(Enumerable) + end + def apply_string_bounds!(schema, field) return unless field[:type] == :string @@ -240,7 +252,7 @@ def annotate(schema, field) schema["writeOnly"] = true schema["x-permittable-sensitive"] = true end - schema["x-permittable-custom-validation"] = true if field[:validate] + schema["x-permittable-custom-validation"] = true if field[:validate] || opaque_in?(field[:in]) schema["x-permittable-transformed"] = true if field[:transform] schema end diff --git a/lib/permittable/rspec.rb b/lib/permittable/rspec.rb index f175ca5..49d883b 100644 --- a/lib/permittable/rspec.rb +++ b/lib/permittable/rspec.rb @@ -231,15 +231,16 @@ def format_mismatch(field, expected) # A contract stores an `in:` list cast by the field's type, so # `within(%i[draft published])` — the declaration repeated as written — - # is cast the same way before comparing, by the same function. A list - # that does not cast (or a Range, kept as written by the contract too) - # is compared as given, and the failure shows both sides. + # is read the same way before comparing, by the same two functions the + # contract uses: what counts as a list (a Hash as its keys), then the + # cast. Anything that is not a list, or does not cast, is compared as + # given — a Range and a host's own allowlist are stored as given too. def cast_in(field, expected) - return expected unless field[:kind] == :scalar && expected.is_a?(Enumerable) && - !expected.is_a?(Range) && !expected.is_a?(Hash) + members = field[:kind] == :scalar && Coercion.in_list(expected) + return expected unless members - status, members = Coercion.cast_in_members(field[:type], expected) - status == :ok ? members : expected + status, cast = Coercion.cast_in_members(field[:type], members, nullable: field[:nullable]) + status == :ok ? cast : expected end def declared_format(field) diff --git a/spec/json_schema_spec.rb b/spec/json_schema_spec.rb index 7a6864f..ed3b34c 100644 --- a/spec/json_schema_spec.rb +++ b/spec/json_schema_spec.rb @@ -75,7 +75,35 @@ def property(name, **opts, &contract) it "exports the cast members, in the field's own JSON type" do expect(property("status") { optional :status, :string, in: %i[draft published] }["enum"]).to eq(%w[draft published]) expect(property("n") { optional :n, :integer, in: %w[1 2 3] }["enum"]).to eq([1, 2, 3]) - expect(property("day") { optional :day, :date, in: ["Sep 5, 2026"] }["enum"]).to eq(["2026-09-05"]) + expect(property("day") { optional :day, :date, in: [Date.new(2026, 9, 5)] }["enum"]).to eq(["2026-09-05"]) + expect(property("status") { optional :status, :string, in: { draft: 0, published: 1 } }["enum"]).to eq(%w[draft published]) + end + + # Re-encoding a cast Time drops what iso8601 does not print: a member + # written with fractional seconds exported as the whole second, which + # the server then refused. A String member is therefore published as + # written, and every published member must be one the server accepts. + it "exports a String-authored :date/:datetime member as written, and the server accepts each one" do + contract = Permittable::Contract.define do + optional :at, :datetime, in: ["2026-09-05T10:00:00.25Z", "2026-09-05T15:00:00+05:00", Time.utc(2026, 1, 1)] + optional :day, :date, in: ["Sep 5, 2026", Date.new(2026, 9, 6)] + end + props = described_class.rule(contract.permittable_contracts.first)["properties"] + expect(props["at"]["enum"]).to eq(["2026-09-05T10:00:00.25Z", "2026-09-05T15:00:00+05:00", "2026-01-01T00:00:00Z"]) + expect(props["day"]["enum"]).to eq(["Sep 5, 2026", "2026-09-06"]) + props.each do |name, schema| + schema["enum"].each do |member| + expect(contract.call(name => member).violations).to be_empty, "#{name}: #{member.inspect} was refused" + end + end + end + + it "exports an :in that only answers include? as custom validation, not as an enum" do + allowlist = Object.new + def allowlist.include?(_value) = true + prop = property("sku") { optional :sku, :string, in: allowlist } + expect(prop).not_to have_key("enum") + expect(prop["x-permittable-custom-validation"]).to be(true) end it "carries a non-numeric Range as an extension instead of guessing" do diff --git a/spec/matchers_spec.rb b/spec/matchers_spec.rb index 11d60d0..6e33fe4 100644 --- a/spec/matchers_spec.rb +++ b/spec/matchers_spec.rb @@ -84,6 +84,20 @@ def failure_of expect(message).to include("declares in: [1, 2, 3]") end + it "reads within's argument exactly as the contract reads in: — a Hash as its keys, an allowlist as itself" do + allowlist = Object.new + def allowlist.include?(_value) = true + contract = Permittable::Contract.define do + optional :status, :string, in: { draft: 0, published: 1 } + optional :sku, :string, in: allowlist + optional :tier, :string, in: [nil, "pro"], nullable: true + end + expect(contract).to permit_param(:status).within({ draft: 0, published: 1 }) + expect(contract).to permit_param(:status).within(%w[draft published]) + expect(contract).to permit_param(:sku).within(allowlist) + expect(contract).to permit_param(:tier).within([nil, "pro"]) + end + it "checks required and optional" do expect(controller).to permit_param(:email).for_action(:create).required expect(controller).to permit_param(:age).for_action(:create).optional diff --git a/spec/permittable_spec.rb b/spec/permittable_spec.rb index 007d26a..4f125c9 100644 --- a/spec/permittable_spec.rb +++ b/spec/permittable_spec.rb @@ -87,19 +87,76 @@ def recording_notifications end end - it "rejects an :in that is neither a Range nor a list of values" do + it "rejects an :in that answers neither cover? nor include?" do expect { permittable_class { permit_params(:create) { required :a, :integer, in: 5 } } } - .to raise_error(ArgumentError, /:in for field :a must be a Range or a list of values .*\(got 5\)/) - # A Hash is Enumerable, but include? asks about its KEYS — not a list. - expect { permittable_class { permit_params(:create) { required :a, :string, in: { "x" => 1 } } } } - .to raise_error(ArgumentError, /:in for field :a must be a Range or a list of values/) + .to raise_error(ArgumentError, /:in for field :a must be a Range, a list of values .*\(got 5\)/) end # String#include? is a substring test: in: "free pro" accepted "e", "fr" # and "ee p" as plans. it "rejects a String :in, which would have matched any substring" do expect { permittable_class { permit_params(:create) { optional :plan, :string, in: "free pro" } } } - .to raise_error(ArgumentError, /:in for field :plan must be a Range or a list of values .*\(got "free pro"\)/) + .to raise_error(ArgumentError, /:in for field :plan must be a Range, a list of values .*\(got "free pro"\).*substring/) + end + + # The Rails enum idiom: `in: Post.statuses` is a HashWithIndifferentAccess + # of name => stored value, and Hash#include? asks about its keys. + it "reads a Hash :in as its keys, cast like any list" do + statuses = ActiveSupport::HashWithIndifferentAccess.new(draft: 0, published: 1) + decl = proc do + permit_params(:create) do + optional :status, :string, in: statuses + optional :tier, :string, in: { free: "f", pro: "p" } + end + end + expect(permit({ status: "published", tier: "pro" }, &decl).to_h).to eq("status" => "published", "tier" => "pro") + expect(violations_for({ status: "0", tier: "f" }, &decl).details) + .to eq([{ param: "status", code: "inclusion" }, { param: "tier", code: "inclusion" }]) + expect(permittable_class(&decl).permit_rule_for(:create)[:fields].map { |f| f[:in] }) + .to eq([%w[draft published], %w[free pro]]) + end + + it "keeps an :in that only answers include? exactly as given, uncast" do + allowlist = Object.new + def allowlist.include?(value) = value.to_s.start_with?("sku-") + decl = proc { permit_params(:create) { optional :sku, :string, in: allowlist } } + expect(permittable_class(&decl).permit_rule_for(:create)[:fields].first[:in]).to be(allowlist) + expect(permit({ sku: "sku-1" }, &decl)[:sku]).to eq("sku-1") + expect(violations_for({ sku: "abc" }, &decl).details).to eq([{ param: "sku", code: "inclusion" }]) + end + + # A lazy list left lazy was cast per request, and the cast's early return + # escaped its block there as a LocalJumpError — a 500. + it "forces a lazy :in to a list once, at class load" do + decl = proc { permit_params(:create) { optional :n, :integer, in: %w[1 2 3].lazy.map(&:itself) } } + field = permittable_class(&decl).permit_rule_for(:create)[:fields].first + expect(field[:in]).to eq([1, 2, 3]).and be_frozen + expect(permit({ n: "2" }, &decl)[:n]).to eq(2) + expect(violations_for({ n: "4" }, &decl).details).to eq([{ param: "n", code: "inclusion" }]) + expect { permittable_class { permit_params(:create) { optional :n, :integer, in: %w[1 x].lazy.map(&:itself) } } } + .to raise_error(ArgumentError, /:in for field :n contains "x"/) + end + + it "reads a Time or DateTime member of a :date field as its date" do + decl = proc do + permit_params(:create) { optional :day, :date, in: [Time.utc(2026, 9, 5, 10), DateTime.new(2026, 9, 6, 23)] } + end + expect(permittable_class(&decl).permit_rule_for(:create)[:fields].first[:in]) + .to eq([Date.new(2026, 9, 5), Date.new(2026, 9, 6)]) + expect(permit({ day: "2026-09-05" }, &decl)[:day]).to eq(Date.new(2026, 9, 5)) + expect(permit({ day: "2026-09-06" }, &decl)[:day]).to eq(Date.new(2026, 9, 6)) + end + + # On a nullable field an explicit null is accepted before in: is ever + # consulted, so a nil member only restates that; elsewhere it is a member + # no request could equal. + it "drops a nil :in member on a nullable field, and refuses it on any other" do + decl = proc { permit_params(:create) { optional :tier, :string, in: [nil, "pro"], nullable: true } } + expect(permittable_class(&decl).permit_rule_for(:create)[:fields].first[:in]).to eq(["pro"]) + expect(permit({ tier: "pro" }, &decl)[:tier]).to eq("pro") + expect(permit({ tier: nil }, &decl).to_h).to eq("tier" => nil) + expect { permittable_class { permit_params(:create) { optional :tier, :string, in: [nil, "pro"] } } } + .to raise_error(ArgumentError, /:in for field :tier contains nil.*declare nullable: true/) end it "rejects an :in member that the field's own type cannot cast" do @@ -107,8 +164,6 @@ def recording_notifications .to raise_error(ArgumentError, /:in for field :n contains "two", which is not a valid :integer \(invalid_type\)/) expect { permittable_class { permit_params(:create) { optional :day, :date, in: ["2026-02-30"] } } } .to raise_error(ArgumentError, /:in for field :day contains "2026-02-30", which is not a valid :date/) - expect { permittable_class { permit_params(:create) { optional :tier, :string, in: [nil, "pro"] } } } - .to raise_error(ArgumentError, /:in for field :tier contains nil.*declare nullable: true/) end it "rejects an :in Range whose endpoints the field's values cannot be compared with" do @@ -2203,6 +2258,15 @@ def duck_model(columns, enums: nil) expect(&declaring(m) { optional :status, :string, in: %w[pending] }).not_to raise_error end + # The guard reads the in: the contract stores — a Hash already read + # as its keys, Symbols already cast to the Strings a request sends — + # so it agrees with what the field will actually accept. + it "accepts the enum's own mapping, or its names as Symbols, as the in:" do + m = enum_model + expect(&declaring(m) { optional :status, :string, in: m.statuses }).not_to raise_error + expect(&declaring(m) { optional :status, :string, in: %i[pending shipped] }).not_to raise_error + end + it "requires the in: — without it, an unknown name would pass and then raise on assignment" do expect(&declaring(enum_model) { optional :status, :string }).to raise_error(ArgumentError) do |e| expect(e.message).to match(/'status' is an enum on EnumThing/) diff --git a/spec/schema_conformance_spec.rb b/spec/schema_conformance_spec.rb index e1a2019..94b2e32 100644 --- a/spec/schema_conformance_spec.rb +++ b/spec/schema_conformance_spec.rb @@ -92,6 +92,15 @@ [{ "n" => "2" }, :coerced_encoding] ] }, + "a :datetime enum written with fractional seconds" => { + # Re-encoding the cast Time printed whole seconds, publishing a member + # the server refused; the String is now published as written. + contract: proc { optional :at, :datetime, in: ["2026-09-05T10:00:00.25Z"] }, + payloads: [ + [{ "at" => "2026-09-05T10:00:00.25Z" }, :agree], + [{ "at" => "2026-09-05T10:00:00Z" }, :agree] + ] + }, "an exclusive range" => { contract: proc { optional :pct, :integer, in: 0...100 }, payloads: [[{ "pct" => 0 }, :agree], [{ "pct" => 99 }, :agree], [{ "pct" => 100 }, :agree]] From cc1ee81848cf0164065a4603d79eee4964d238f4 Mon Sep 17 00:00:00 2001 From: Sang Date: Fri, 25 Sep 2026 14:22:15 +0700 Subject: [PATCH 3/4] Cast only plain lists, keep hash keys a Set, and export exact instants Regression-pass follow-ups on the class-load in: check: - Only Array, Set, Hash (keys) and Enumerator are lists to cast. Any other object answering include?, Enumerable or not, is used as given, so an app's case-insensitive allowlist or DB-backed registry keeps its own include? and is never enumerated at class load. - A Hash's keys (and a Set) are stored as a frozen Set: O(1) membership. The matcher compares lists as sets accordingly. - A Time/DateTime member of a :date field is read as its UTC date only at exactly midnight UTC, the one instant that ever equalled a date; any other fails at class load. - An exported Time keeps its fractional-second digits (up to nine), so every exported :datetime member is one the server accepts. - CHANGELOG: the class-load snapshot of an Array is listed as a change, and the newly-failing declarations are described precisely. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 17 +++++++----- README.md | 4 +-- lib/permittable.rb | 45 ++++++++++++++++++++---------- lib/permittable/json_schema.rb | 19 ++++++++++--- lib/permittable/rspec.rb | 10 ++++++- spec/json_schema_spec.rb | 23 ++++++++++++++++ spec/permittable_spec.rb | 50 +++++++++++++++++++++++++++++++--- 7 files changed, 136 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c2248b..c59aafe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,16 +61,19 @@ Contracts that don't opt in are byte-for-byte unaffected: the default format is - **Enum columns drafted as `:integer`**, rejecting the `"shipped"` every form sends. A Rails `enum` now drafts as `:string, in: Model.statuses.keys` — the model's own accessor rather than today's keys inlined, so adding a value cannot leave the contract behind — and a database default is shown as its enum key (`"pending"`, not `0`). Rails also assigns an integer-backed enum its stored integer (`status: 1` from a JSON client), which a `:string` field turns into a rejected `"1"`; the line carries a TODO saying how to admit it rather than guessing that clients send it. An enum whose name is not a method identifier (`first-status`) is reached as `Model.defined_enums["first-status"]`, so the draft still loads. - **The STI inheritance column and `lock_version` were drafted as client-writable fields.** Mass-assigning `type` changes which class the record loads as, which is a privilege-escalation shape, not a field. Drafted from columns alone, both are now omitted from the fields and named in a TODO explaining why — including where to declare `lock_version` if the app's forms round-trip it for stale-update detection, worded for the rules actually drafted. When the controller's own permit call lists one, it stays a field with a TODO instead: omitting a `lock_version` the app sends would switch stale-update detection off the day the draft is enforced. Each counts only when the model uses it — a `type` column with `self.inheritance_column = nil`, or `lock_version` with `self.lock_optimistically = false`, is an ordinary column and drafts as one. A model left with no field to draft (only `type` and `lock_version`, or nothing else with a contract type) drafts nothing, as a model with no columns does, rather than a rule that raises `a contract must declare at least one field` when pasted. - **Column names that are not symbol literals produced a draft that did not parse.** `first-name`, `2fa_enabled` and `Email Address` were emitted as `:first-name` and friends; names are now emitted with `Symbol#inspect` (`:"first-name"`), so the draft is valid Ruby whatever the schema. -- **`in:` members are now cast with the field's own type, at class load.** The runtime compared the *cast* request value against the members *as authored*, so `in: %i[draft published]` on a `:string` field compared `"draft"` with `:draft` and answered `inclusion` to **every** request — while the exported schema, which stringifies Symbols, advertised `"enum": ["draft", "published"]`, the very values the server refused. `in: %w[1 2 3]` on an `:integer` field rejected every value the same way. Each member now goes through the same cast a request value does (a Symbol read as its String, a `Time`/`DateTime` on a `:date` field read as its date; `normalize:` not applied, since it rewrites what a client sent rather than what the contract says), and the contract stores the cast members — frozen, deduplicated, and still a `Set` if one was given. A `Hash` is read as its keys, which is what `Hash#include?` always asked about, so the Rails enum idiom `in: Post.statuses` keeps working — and now satisfies `check_column_types`' enum rule, which reads the stored keys. Any other list, an `Enumerator::Lazy` included, is forced to an Array once at class load rather than cast per request. Request-time matching, the exported `enum` (which for `%w[1 2 3]` on an `:integer` is now `[1, 2, 3]`, not `["1", "2", "3"]`), the column guard and the RSpec matcher all read that one list; `within` reads its own argument with the same two functions, so `within(%i[draft published])` can repeat the declaration as written. A `nil` member is dropped on a `nullable:` field, where an explicit null is accepted before `in:` is consulted. -- **A `:date`/`:datetime` `in:` member written as a String is exported as written.** Re-encoding the cast `Time` printed whole seconds, so `in: ["2026-09-05T10:00:00.25Z"]` published `"2026-09-05T10:00:00Z"`, a value the server refused. The authored String went through the very cast a request does, so every published member is one the server accepts, and a spec holds each one to that. -- **`in:` can no longer be a `String`.** The only check was `respond_to?(:include?)`, which a `String` passes — and `String#include?` is a substring test, so `in: "free pro"` accepted `"e"`, `"fr"` and `"ee p"` as plans. A `String` now fails at class load, as anything answering neither `cover?` nor `include?` already did. An object of your own that answers `include?` is still used exactly as given — not cast, and exported as `x-permittable-custom-validation` rather than an `enum` it cannot list. +- **`in:` members are now cast with the field's own type, at class load.** The runtime compared the *cast* request value against the members *as authored*, so `in: %i[draft published]` on a `:string` field compared `"draft"` with `:draft` and answered `inclusion` to **every** request — while the exported schema, which stringifies Symbols, advertised `"enum": ["draft", "published"]`, the very values the server refused. `in: %w[1 2 3]` on an `:integer` field rejected every value the same way. Each member of a **list** — an `Array`, `Set`, `Enumerator` (`Lazy` included, forced once rather than cast per request), or a `Hash` read as its keys, which is what `Hash#include?` always asked about — now goes through the same cast a request value does: a Symbol is read as its String, and `normalize:` is not applied, since it rewrites what a client sent rather than what the contract says. The contract stores the cast members frozen and deduplicated; a `Set` or a `Hash`'s keys are stored as a `Set`, so membership stays O(1) per request. The Rails enum idiom `in: Post.statuses` therefore keeps working, and satisfies `check_column_types`' enum rule, which reads the stored keys. Request-time matching, the exported `enum` (which for `%w[1 2 3]` on an `:integer` is now `[1, 2, 3]`, not `["1", "2", "3"]`), the column guard and the RSpec matcher all read that one list; `within` reads its own argument with the same two functions, and compares lists as sets, so `within(%i[draft published])` can repeat the declaration as written. A `nil` member is dropped on a `nullable:` field, where an explicit null is accepted before `in:` is consulted. A `Time` or `DateTime` member of a `:date` field is read as its date when it is exactly midnight UTC — the one instant ActiveSupport ever found equal to a date. +- **Any other object answering `include?` is still used exactly as given**, Enumerable or not — an app's own case-insensitive allowlist, or a DB-backed registry, is never enumerated at class load nor replaced by an exact-match copy. It is not cast, and it is exported as `x-permittable-custom-validation` rather than an `enum` it cannot list. +- **Every exported `:date`/`:datetime` member is one the server accepts.** A member written as a String is published as written, and a `Time`/`DateTime`/`TimeWithZone` member is published with as many fractional-second digits as it has (up to nine). Re-encoding printed whole seconds, so `in: ["2026-09-05T10:00:00.25Z"]` published `"2026-09-05T10:00:00Z"`, a value the server refused. The same encoding applies to an exported `:datetime` `default:`/`example:`. A spec sends every exported member back through the contract. +- **`in:` can no longer be a `String`.** The only check was `respond_to?(:include?)`, which a `String` passes — and `String#include?` is a substring test, so `in: "free pro"` accepted `"e"`, `"fr"` and `"ee p"` as plans. A `String` now fails at class load, as anything answering neither `cover?` nor `include?` already did. - **An `in:` `Range` the field's values cannot be compared with fails at class load.** `in: "1".."5"` on an `:integer`, or `in: 1..5` on a `:string`, made `cover?` answer `false` for every value. A Range is deliberately **not** cast — casting would change what it means (`0..Float::INFINITY` on a `:float` and `1.5..3` on an `:integer` are real bounds no cast accepts, and a `:decimal`'s `0..100` would export its `minimum` as the string `"0.0"`) — so it is kept exactly as written, and refused only when its endpoints cannot be compared with a value of the field's type, asked the way `cover?` itself asks. -The cast only **loosens** contracts: no request a contract accepted before is refused now. Declarations that used to boot and now fail at class load, each of which answered `inclusion` to every request (or, for a `String`, let fragments through): -- a `String` `in:` — write it as a list, `in: %w[free pro]`; -- a list member the field's type cannot cast (`in: %w[1 two]` on an `:integer`), including `nil` on a field that isn't `nullable:` — the error names `nullable: true`; +**A list is now a snapshot taken at class load.** Until now an `in:` Array was read live, so a constant mutated after the class loaded — `PLANS << "gold"` in an initializer — was seen by later requests. It is now cast and frozen once, so that mutation is not seen. Declare the full list before the contract loads, or pass an object of your own answering `include?`, which is still read on every request. + +The cast only **loosens** what a request is judged against: no request a contract accepted before is refused now. These declarations used to boot and now fail at class load, each because something in it could never match — though a list's other, valid members did match before, so a contract such as `in: [1, 2, "three"]` was partly working, not wholly broken: +- a `String` `in:` — it matched substrings; write it as a list, `in: %w[free pro]`; +- a list member the field's type cannot cast (`"three"` in `in: [1, 2, "three"]` on an `:integer`), including `nil` on a field that isn't `nullable:` (the error names `nullable: true`), and a `Time`/`DateTime` member of a `:date` field that is not exactly midnight UTC; - `in: [nil]` on a `nullable:` field, which lists nothing once the `nil` is dropped (an explicit null needs no `in:`); -- a `Range` whose endpoints the field's values cannot be compared with (`in: "1".."5"` on an `:integer`). +- a `Range` whose endpoints the field's values cannot be compared with (`in: "1".."5"` on an `:integer`) — this one did reject every value. One exported-docs change for lists that already worked: the `enum` now carries the field's own encoding, so `in: [1.5]` on a `:decimal` exports `"1.5"` (the same precision-safe string its `default:` already exports) and `in: [1, 2]` on a `:float` exports `1.0, 2.0`. diff --git a/README.md b/README.md index 8592dcd..4cd595f 100644 --- a/README.md +++ b/README.md @@ -292,7 +292,7 @@ Which options are legal depends on the field kind — anything else raises at cl | Option | Scalar | Array | Nested | Meaning | |---|:---:|:---:|:---:|---| -| `in:` | ✅ | — | — | Allowed values: a `Range` (bounds-checked with `cover?`), a list (`Array`, `Set`, or a `Hash` read as its keys — so `in: Post.statuses` works), or your own object answering `include?` (used as given). List members are cast with the field's own type at class load, so `in: %i[draft published]` on a `:string` and `in: %w[1 2 3]` on an `:integer` match what a request casts to; a `nil` member is dropped on a `nullable:` field | +| `in:` | ✅ | — | — | Allowed values: a `Range` (bounds-checked with `cover?`), a list (`Array`, `Set`, `Enumerator`, or a `Hash` read as its keys — so `in: Post.statuses` works), or any other object answering `include?`, Enumerable or not (used as given, and read on every request). A list is cast with the field's own type and **snapshotted** at class load, so `in: %i[draft published]` on a `:string` and `in: %w[1 2 3]` on an `:integer` match what a request casts to — and a later `PLANS << "gold"` is not seen; pass your own `include?` object for a live list. A `nil` member is dropped on a `nullable:` field | | `format:` | ✅¹ | — | — | Regexp the value must match, or a [preset name](#format-presets): `:email`, `:uuid`, `:url`, `:slug`, `:hostname` | | `length:` | ✅¹ | ✅ | — | `Range` or `Integer`. Character count on strings, **element count** on arrays, where it short-circuits — see [the field DSL](#the-field-dsl) | | `normalize:` | ✅¹ | — | — | `:squish`, `:strip`, `:downcase`, `:upcase`, `:email`, or a Proc. Runs **first** — before the absence rule, so a value that normalizes to `""` is absent | @@ -1073,7 +1073,7 @@ A bad contract is a programmer error, so it fails when the class loads — never - A `format:` that is neither a `Regexp` nor a preset name - `format:`, `length:`, or `normalize:` on a non-`:string` field - `length:` that isn't a non-negative `Integer` or a `Range`; an `in:` that is a `String` (`String#include?` would match any substring — `in: "free pro"` accepted `"e"`), or that answers neither `cover?` nor `include?` -- An `in:` member the field's own type can't cast (`in: %w[1 two]` on an `:integer`, or `nil` on a field that isn't `nullable:`), or an `in:` `Range` whose endpoints a value of the field's type can't be compared with (`in: "1".."5"` on an `:integer`) — either would reject every request as `inclusion` +- An `in:` member the field's own type can't cast (`in: %w[1 two]` on an `:integer`, `nil` on a field that isn't `nullable:`, or a `Time` on a `:date` field that isn't exactly midnight UTC), or an `in:` `Range` whose endpoints a value of the field's type can't be compared with (`in: "1".."5"` on an `:integer`) — either would reject every request as `inclusion` - A bound **no value could satisfy**: a reversed or empty `Range` (`in: 65..18`, `length: 5..2`, `length: 3...3`), an empty `in:` set, or a `length:` of 0 on a `required` field (where `""` already violates as `missing`) - `validate:` or `transform:` that isn't callable - A `default:` or `example:` that violates its own field's contract, or an array `default:`/`example:` whose elements violate `of:` — or, for an array declared with a **block**, an element that isn't a hash the block would accept diff --git a/lib/permittable.rb b/lib/permittable.rb index aeb151d..0337b3c 100644 --- a/lib/permittable.rb +++ b/lib/permittable.rb @@ -744,31 +744,48 @@ def cast_in_members(type, members, nullable: false) [:ok, members.is_a?(Set) ? cast_members.to_set : cast_members, pairs.map(&:last)] end - # The members of an `in:` that is a LIST, or nil when it is not one: a - # Range bounds rather than lists, and an object that merely answers - # include? (a host's own allowlist) is kept as given. A Hash lists its - # KEYS, which is what Hash#include? asks about — the Rails enum idiom, - # `in: Post.statuses`. Anything else Enumerable is forced to an Array - # here, once: an Enumerator::Lazy left lazy would be cast on every + # The members of an `in:` that is a LIST, or nil when it is not one. + # Only the collections whose include? is plain membership count: Array, + # Set, Hash and Enumerator. Anything else answering include? — a Range, + # or an app's own object, Enumerable or not — is used as given, because + # its include? may be exactly the point (a case-insensitive allowlist) + # and enumerating it may be expensive (a DB-backed registry). + # + # A Hash lists its KEYS, which is what Hash#include? asks about — the + # Rails enum idiom, `in: Post.statuses` — and, like a Set, stays a Set, + # so membership stays O(1) per request. An Enumerator (Lazy included) is + # forced to an Array here, once: left lazy, it would be cast on every # request instead of at class load. def in_list(allowed) - return nil if allowed.is_a?(Range) || !allowed.is_a?(Enumerable) - return allowed.keys if allowed.is_a?(Hash) - - allowed.is_a?(Set) ? allowed : allowed.to_a + case allowed + when Hash then allowed.keys.to_set + when Set then allowed + when Array, Enumerator then allowed.to_a + end end # A Symbol is read as its String: it is how Ruby spells a constant # string, and a request never carries one, so no cast accepts it as is. - # A Time or DateTime on a :date field is read as its date — cast_date - # would keep a DateTime whole (it IS a Date) and refuse a Time, and - # neither would ever equal the Date a request casts to. def cast_in_member(type, member) member = member.to_s if member.is_a?(Symbol) - member = member.to_date if type == :date && (member.is_a?(Time) || member.is_a?(DateTime)) + return instant_as_date(member) if type == :date && (member.is_a?(Time) || member.is_a?(DateTime)) + cast(type, member) end + # A Time or DateTime member of a :date field. ActiveSupport compares one + # with a Date as INSTANTS, the Date standing for its midnight UTC, so + # that instant is the only one that ever equalled a request's date. It + # is read as that UTC date; any other instant never matched anything, + # and is refused like any member no request could equal. (cast_date + # would keep a DateTime whole — it IS a Date — and refuse a Time.) + def instant_as_date(member) + utc = member.to_time.getutc + return [:error, "not midnight UTC, so it never equals a date"] unless utc == utc.beginning_of_day + + [:ok, utc.to_date] + end + # What an exported enum lists for one member: the cast value, re-encoded # as JSON — except a :date/:datetime member authored as a String, which # is published AS WRITTEN. Re-encoding a cast Time prints whole seconds, diff --git a/lib/permittable/json_schema.rb b/lib/permittable/json_schema.rb index 75f70d3..753428b 100644 --- a/lib/permittable/json_schema.rb +++ b/lib/permittable/json_schema.rb @@ -182,9 +182,10 @@ def apply_in!(schema, field) schema[allowed.exclude_end? ? "exclusiveMaximum" : "maximum"] = json_value(allowed.end) if allowed.end end - # A host's own include?-answering allowlist, kept by the contract as given. + # A host's own include?-answering object, kept by the contract as given — + # the same predicate the contract used to decide it was not a list. def opaque_in?(allowed) - !allowed.nil? && !allowed.is_a?(Range) && !allowed.is_a?(Enumerable) + !allowed.nil? && !allowed.is_a?(Range) && Coercion.in_list(allowed).nil? end def apply_string_bounds!(schema, field) @@ -264,13 +265,23 @@ def json_value(value) # the same re-encoding as any other authored scalar. when Hash then value.to_h { |k, v| [k.to_s, json_value(v)] } when BigDecimal then value.to_s("F") - when Time then value.utc.iso8601 + when Time then exact_iso8601(value.getutc) # DateTime subclasses Date, so it must match first. - when DateTime then value.to_time.utc.iso8601 + when DateTime then exact_iso8601(value.to_time.getutc) when Date then value.iso8601 when Symbol then value.to_s else value end end + + # iso8601 prints whole seconds unless told otherwise, and a sub-second + # instant re-encoded that way names a DIFFERENT instant — one an `in:` + # listing the original refuses. So as many fractional digits as the + # value has, up to the nanoseconds Time#nsec can report. + def exact_iso8601(time) + nsec = time.nsec + digits = nsec.zero? ? 0 : 9 - nsec.to_s.rjust(9, "0")[/0*\z/].length + time.iso8601(digits) + end end end diff --git a/lib/permittable/rspec.rb b/lib/permittable/rspec.rb index 49d883b..f9af375 100644 --- a/lib/permittable/rspec.rb +++ b/lib/permittable/rspec.rb @@ -198,7 +198,7 @@ def check_mismatch(field, key, value) when :required then required_mismatch(field, value) when :format then format_mismatch(field, value) # Compared cast, but reported as written. - when :in then option_mismatch(field, :in, value) unless field.key?(:in) && field[:in] == cast_in(field, value) + when :in then option_mismatch(field, :in, value) unless field.key?(:in) && same_in?(field[:in], cast_in(field, value)) when :virtual, :sensitive, :nullable then "expected the field to be #{key}, but it is not" unless field[key] else option_mismatch(field, key, value) end @@ -243,6 +243,14 @@ def cast_in(field, expected) status == :ok ? cast : expected end + # A list's order and container say nothing about what it allows: + # `in: Post.statuses` is stored as a Set, and `within(%w[draft + # published])` names exactly its values. + def same_in?(declared, expected) + lists = [declared, expected].all? { |list| list.is_a?(Array) || list.is_a?(Set) } + lists ? declared.to_set == expected.to_set : declared == expected + end + def declared_format(field) return "declares format: :#{field[:format_name]}" if field[:format_name] return "declares format: #{field[:format].inspect}" if field[:format] diff --git a/spec/json_schema_spec.rb b/spec/json_schema_spec.rb index ed3b34c..abf5ded 100644 --- a/spec/json_schema_spec.rb +++ b/spec/json_schema_spec.rb @@ -98,12 +98,35 @@ def property(name, **opts, &contract) end end + # A Time/DateTime/TimeWithZone member is re-encoded, so it must keep the + # sub-second digits it has — whole seconds named an instant the server + # refused. + it "exports a sub-second Time-like :datetime member with its fractional digits, and the server accepts it" do + members = [Time.utc(2026, 9, 5, 10, 0, Rational(1, 4)), DateTime.new(2026, 9, 5, 11, 0, Rational(123_456_789, 10**9)), + Time.utc(2026, 9, 5, 12).in_time_zone("Tokyo") + Rational(1, 1000), Time.utc(2026, 9, 5, 13)] + contract = Permittable::Contract.define { optional :at, :datetime, in: members } + enum = described_class.rule(contract.rule)["properties"]["at"]["enum"] + expect(enum).to eq(["2026-09-05T10:00:00.25Z", "2026-09-05T11:00:00.123456789Z", + "2026-09-05T12:00:00.001Z", "2026-09-05T13:00:00Z"]) + enum.each { |member| expect(contract.call(at: member).violations).to be_empty, "#{member} was refused" } + end + it "exports an :in that only answers include? as custom validation, not as an enum" do allowlist = Object.new def allowlist.include?(_value) = true prop = property("sku") { optional :sku, :string, in: allowlist } expect(prop).not_to have_key("enum") expect(prop["x-permittable-custom-validation"]).to be(true) + + plans = Class.new do + include Enumerable + + def each(&) = %w[free pro].each(&) + def include?(value) = %w[free pro].include?(value.to_s.downcase) + end.new + prop = property("plan") { optional :plan, :string, in: plans } + expect(prop).not_to have_key("enum") + expect(prop["x-permittable-custom-validation"]).to be(true) end it "carries a non-numeric Range as an extension instead of guessing" do diff --git a/spec/permittable_spec.rb b/spec/permittable_spec.rb index 4f125c9..7146990 100644 --- a/spec/permittable_spec.rb +++ b/spec/permittable_spec.rb @@ -112,8 +112,10 @@ def recording_notifications expect(permit({ status: "published", tier: "pro" }, &decl).to_h).to eq("status" => "published", "tier" => "pro") expect(violations_for({ status: "0", tier: "f" }, &decl).details) .to eq([{ param: "status", code: "inclusion" }, { param: "tier", code: "inclusion" }]) - expect(permittable_class(&decl).permit_rule_for(:create)[:fields].map { |f| f[:in] }) - .to eq([%w[draft published], %w[free pro]]) + # A Set, so membership stays O(1) per request as Hash#include? was. + ins = permittable_class(&decl).permit_rule_for(:create)[:fields].map { |f| f[:in] } + expect(ins).to eq([Set["draft", "published"], Set["free", "pro"]]) + expect(ins).to all(be_frozen) end it "keeps an :in that only answers include? exactly as given, uncast" do @@ -125,6 +127,35 @@ def allowlist.include?(value) = value.to_s.start_with?("sku-") expect(violations_for({ sku: "abc" }, &decl).details).to eq([{ param: "sku", code: "inclusion" }]) end + # Only Array, Set, Hash and Enumerator are lists. An app's own Enumerable + # with its own include? — a case-insensitive allowlist, a DB-backed + # registry — is used as given: never enumerated at class load, never + # replaced by an exact-match copy. + it "keeps an app's own Enumerable that defines include? as given, never enumerating it" do + plans = Class.new do + include Enumerable + + def each = raise("enumerated at class load") + def include?(value) = %w[free pro].include?(value.to_s.downcase) + end.new + decl = proc { permit_params(:create) { optional :plan, :string, in: plans } } + expect(permittable_class(&decl).permit_rule_for(:create)[:fields].first[:in]).to be(plans) + expect(permit({ plan: "PRO" }, &decl)[:plan]).to eq("PRO") + expect(violations_for({ plan: "gold" }, &decl).details).to eq([{ param: "plan", code: "inclusion" }]) + end + + # The approved snapshot: a list is cast once, so a later `PLANS << "gold"` + # is not seen. An app that needs a live list passes its own include? + # object, which is read on every request. + it "snapshots an Array :in at class load" do + plans = %w[free pro] + klass = permittable_class { permit_params(:create) { optional :plan, :string, in: plans } } + plans << "gold" + e = klass.new(params: { plan: "gold" }) + e.define_singleton_method(:action_name) { "create" } + expect(e.permittable_violations).to eq([{ param: "plan", code: "inclusion" }]) + end + # A lazy list left lazy was cast per request, and the cast's early return # escaped its block there as a LocalJumpError — a 500. it "forces a lazy :in to a list once, at class load" do @@ -137,14 +168,25 @@ def allowlist.include?(value) = value.to_s.start_with?("sku-") .to raise_error(ArgumentError, /:in for field :n contains "x"/) end - it "reads a Time or DateTime member of a :date field as its date" do + # ActiveSupport compares a Time (or DateTime) with a Date as instants, + # the Date standing for its midnight UTC — so that instant was the only + # one that ever matched. It is read as that UTC date; any other instant + # never matched a request, and fails like any never-matching member. + it "reads a Time or DateTime member of a :date field as its date only at midnight UTC" do decl = proc do - permit_params(:create) { optional :day, :date, in: [Time.utc(2026, 9, 5, 10), DateTime.new(2026, 9, 6, 23)] } + permit_params(:create) do + optional :day, :date, in: [Time.utc(2026, 9, 5), DateTime.new(2026, 9, 6, 5, 0, 0, "+05:00")] + end end expect(permittable_class(&decl).permit_rule_for(:create)[:fields].first[:in]) .to eq([Date.new(2026, 9, 5), Date.new(2026, 9, 6)]) expect(permit({ day: "2026-09-05" }, &decl)[:day]).to eq(Date.new(2026, 9, 5)) expect(permit({ day: "2026-09-06" }, &decl)[:day]).to eq(Date.new(2026, 9, 6)) + + [Time.utc(2026, 9, 5, 10), Time.new(2026, 9, 5, 0, 0, 0, "+05:00"), DateTime.new(2026, 9, 6, 23)].each do |member| + expect { permittable_class { permit_params(:create) { optional :day, :date, in: [member] } } } + .to raise_error(ArgumentError, /:in for field :day contains .*, which is not a valid :date \(not midnight UTC/) + end end # On a nullable field an explicit null is accepted before in: is ever From ff7891fc0d3de2ae28f958a59f8dbd5bc83e207a Mon Sep 17 00:00:00 2001 From: Sang Date: Fri, 25 Sep 2026 15:30:09 +0700 Subject: [PATCH 4/4] Keep a Hash/Array/Set subclass's own include? override, not its contents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit case/when dispatch on the collection's own class used === (is_a? for a Class), so a subclass overriding include? for its own matching logic (a case-insensitive allowlist, a fuzzy Set, a registry) matched its ancestor's branch and had its override silently discarded, read for its raw keys/elements instead — inverting which values it accepts, with no error at class load. Only a plain Array, Set, Hash, or Enumerator (an include? that is not overridden) is now read as a list. ActiveSupport::HashWithIndifferentAccess is kept as one anyway, since its own override just canonicalises the argument before the same key lookup and it is what a Rails enum's own reader (Post.statuses) actually returns. Also tightens the README/CHANGELOG wording that implied cover? duck- typing on a non-Range in:, which resolve_in! never did. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 4 +-- README.md | 4 +-- lib/permittable.rb | 42 +++++++++++++++++++++--------- spec/json_schema_spec.rb | 20 +++++++++++++++ spec/matchers_spec.rb | 7 +++++ spec/permittable_spec.rb | 55 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 116 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c59aafe..bbe03c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,9 +62,9 @@ Contracts that don't opt in are byte-for-byte unaffected: the default format is - **The STI inheritance column and `lock_version` were drafted as client-writable fields.** Mass-assigning `type` changes which class the record loads as, which is a privilege-escalation shape, not a field. Drafted from columns alone, both are now omitted from the fields and named in a TODO explaining why — including where to declare `lock_version` if the app's forms round-trip it for stale-update detection, worded for the rules actually drafted. When the controller's own permit call lists one, it stays a field with a TODO instead: omitting a `lock_version` the app sends would switch stale-update detection off the day the draft is enforced. Each counts only when the model uses it — a `type` column with `self.inheritance_column = nil`, or `lock_version` with `self.lock_optimistically = false`, is an ordinary column and drafts as one. A model left with no field to draft (only `type` and `lock_version`, or nothing else with a contract type) drafts nothing, as a model with no columns does, rather than a rule that raises `a contract must declare at least one field` when pasted. - **Column names that are not symbol literals produced a draft that did not parse.** `first-name`, `2fa_enabled` and `Email Address` were emitted as `:first-name` and friends; names are now emitted with `Symbol#inspect` (`:"first-name"`), so the draft is valid Ruby whatever the schema. - **`in:` members are now cast with the field's own type, at class load.** The runtime compared the *cast* request value against the members *as authored*, so `in: %i[draft published]` on a `:string` field compared `"draft"` with `:draft` and answered `inclusion` to **every** request — while the exported schema, which stringifies Symbols, advertised `"enum": ["draft", "published"]`, the very values the server refused. `in: %w[1 2 3]` on an `:integer` field rejected every value the same way. Each member of a **list** — an `Array`, `Set`, `Enumerator` (`Lazy` included, forced once rather than cast per request), or a `Hash` read as its keys, which is what `Hash#include?` always asked about — now goes through the same cast a request value does: a Symbol is read as its String, and `normalize:` is not applied, since it rewrites what a client sent rather than what the contract says. The contract stores the cast members frozen and deduplicated; a `Set` or a `Hash`'s keys are stored as a `Set`, so membership stays O(1) per request. The Rails enum idiom `in: Post.statuses` therefore keeps working, and satisfies `check_column_types`' enum rule, which reads the stored keys. Request-time matching, the exported `enum` (which for `%w[1 2 3]` on an `:integer` is now `[1, 2, 3]`, not `["1", "2", "3"]`), the column guard and the RSpec matcher all read that one list; `within` reads its own argument with the same two functions, and compares lists as sets, so `within(%i[draft published])` can repeat the declaration as written. A `nil` member is dropped on a `nullable:` field, where an explicit null is accepted before `in:` is consulted. A `Time` or `DateTime` member of a `:date` field is read as its date when it is exactly midnight UTC — the one instant ActiveSupport ever found equal to a date. -- **Any other object answering `include?` is still used exactly as given**, Enumerable or not — an app's own case-insensitive allowlist, or a DB-backed registry, is never enumerated at class load nor replaced by an exact-match copy. It is not cast, and it is exported as `x-permittable-custom-validation` rather than an `enum` it cannot list. +- **Any other object answering `include?` is still used exactly as given**, Enumerable or not — an app's own case-insensitive allowlist, or a DB-backed registry, is never enumerated at class load nor replaced by an exact-match copy. This includes a `Hash`/`Array`/`Set` **subclass that overrides `include?`**: `case allowed; when Hash ...` matches with `===`, which for a Class is `is_a?`, so a subclass first matched its ancestor's branch and had its override silently discarded, read for its raw keys/elements instead — inverting which values it actually accepted, with no error at class load. Only a plain `Array`, `Set`, `Hash`, or `Enumerator` (an unoverridden `include?`) is now read as a list; `ActiveSupport::HashWithIndifferentAccess` is kept as one anyway, since its own override only canonicalises the argument before the same key lookup, and it is what a Rails enum's own reader (`Post.statuses`) actually returns. Any other override is not cast, and is exported as `x-permittable-custom-validation` rather than an `enum` it cannot list. - **Every exported `:date`/`:datetime` member is one the server accepts.** A member written as a String is published as written, and a `Time`/`DateTime`/`TimeWithZone` member is published with as many fractional-second digits as it has (up to nine). Re-encoding printed whole seconds, so `in: ["2026-09-05T10:00:00.25Z"]` published `"2026-09-05T10:00:00Z"`, a value the server refused. The same encoding applies to an exported `:datetime` `default:`/`example:`. A spec sends every exported member back through the contract. -- **`in:` can no longer be a `String`.** The only check was `respond_to?(:include?)`, which a `String` passes — and `String#include?` is a substring test, so `in: "free pro"` accepted `"e"`, `"fr"` and `"ee p"` as plans. A `String` now fails at class load, as anything answering neither `cover?` nor `include?` already did. +- **`in:` can no longer be a `String`.** The only check was `respond_to?(:include?)`, which a `String` passes — and `String#include?` is a substring test, so `in: "free pro"` accepted `"e"`, `"fr"` and `"ee p"` as plans. A `String` now fails at class load, as anything that is neither a `Range` nor answers `include?` already did. - **An `in:` `Range` the field's values cannot be compared with fails at class load.** `in: "1".."5"` on an `:integer`, or `in: 1..5` on a `:string`, made `cover?` answer `false` for every value. A Range is deliberately **not** cast — casting would change what it means (`0..Float::INFINITY` on a `:float` and `1.5..3` on an `:integer` are real bounds no cast accepts, and a `:decimal`'s `0..100` would export its `minimum` as the string `"0.0"`) — so it is kept exactly as written, and refused only when its endpoints cannot be compared with a value of the field's type, asked the way `cover?` itself asks. **A list is now a snapshot taken at class load.** Until now an `in:` Array was read live, so a constant mutated after the class loaded — `PLANS << "gold"` in an initializer — was seen by later requests. It is now cast and frozen once, so that mutation is not seen. Declare the full list before the contract loads, or pass an object of your own answering `include?`, which is still read on every request. diff --git a/README.md b/README.md index 4cd595f..a5686d1 100644 --- a/README.md +++ b/README.md @@ -292,7 +292,7 @@ Which options are legal depends on the field kind — anything else raises at cl | Option | Scalar | Array | Nested | Meaning | |---|:---:|:---:|:---:|---| -| `in:` | ✅ | — | — | Allowed values: a `Range` (bounds-checked with `cover?`), a list (`Array`, `Set`, `Enumerator`, or a `Hash` read as its keys — so `in: Post.statuses` works), or any other object answering `include?`, Enumerable or not (used as given, and read on every request). A list is cast with the field's own type and **snapshotted** at class load, so `in: %i[draft published]` on a `:string` and `in: %w[1 2 3]` on an `:integer` match what a request casts to — and a later `PLANS << "gold"` is not seen; pass your own `include?` object for a live list. A `nil` member is dropped on a `nullable:` field | +| `in:` | ✅ | — | — | Allowed values: a `Range` (bounds-checked with `cover?`), a list (a plain `Array`, `Set`, `Enumerator`, or `Hash` read as its keys — so `in: Post.statuses` works), or any other object answering `include?` (used as given, and read on every request) — including a Hash/Array/Set **subclass that overrides `include?`**, whose override is kept rather than read for its raw contents. A list is cast with the field's own type and **snapshotted** at class load, so `in: %i[draft published]` on a `:string` and `in: %w[1 2 3]` on an `:integer` match what a request casts to — and a later `PLANS << "gold"` is not seen; pass your own `include?` object for a live list. A `nil` member is dropped on a `nullable:` field | | `format:` | ✅¹ | — | — | Regexp the value must match, or a [preset name](#format-presets): `:email`, `:uuid`, `:url`, `:slug`, `:hostname` | | `length:` | ✅¹ | ✅ | — | `Range` or `Integer`. Character count on strings, **element count** on arrays, where it short-circuits — see [the field DSL](#the-field-dsl) | | `normalize:` | ✅¹ | — | — | `:squish`, `:strip`, `:downcase`, `:upcase`, `:email`, or a Proc. Runs **first** — before the absence rule, so a value that normalizes to `""` is absent | @@ -1072,7 +1072,7 @@ A bad contract is a programmer error, so it fails when the class loads — never - An unknown `normalize:` or `format:` preset, listing the presets - A `format:` that is neither a `Regexp` nor a preset name - `format:`, `length:`, or `normalize:` on a non-`:string` field -- `length:` that isn't a non-negative `Integer` or a `Range`; an `in:` that is a `String` (`String#include?` would match any substring — `in: "free pro"` accepted `"e"`), or that answers neither `cover?` nor `include?` +- `length:` that isn't a non-negative `Integer` or a `Range`; an `in:` that is a `String` (`String#include?` would match any substring — `in: "free pro"` accepted `"e"`), or that is neither a `Range` nor answers `include?` - An `in:` member the field's own type can't cast (`in: %w[1 two]` on an `:integer`, `nil` on a field that isn't `nullable:`, or a `Time` on a `:date` field that isn't exactly midnight UTC), or an `in:` `Range` whose endpoints a value of the field's type can't be compared with (`in: "1".."5"` on an `:integer`) — either would reject every request as `inclusion` - A bound **no value could satisfy**: a reversed or empty `Range` (`in: 65..18`, `length: 5..2`, `length: 3...3`), an empty `in:` set, or a `length:` of 0 on a `required` field (where `""` already violates as `missing`) - `validate:` or `transform:` that isn't callable diff --git a/lib/permittable.rb b/lib/permittable.rb index 0337b3c..5f2cff6 100644 --- a/lib/permittable.rb +++ b/lib/permittable.rb @@ -745,25 +745,43 @@ def cast_in_members(type, members, nullable: false) end # The members of an `in:` that is a LIST, or nil when it is not one. - # Only the collections whose include? is plain membership count: Array, - # Set, Hash and Enumerator. Anything else answering include? — a Range, - # or an app's own object, Enumerable or not — is used as given, because - # its include? may be exactly the point (a case-insensitive allowlist) - # and enumerating it may be expensive (a DB-backed registry). + # Only Array, Set, Hash and Enumerator count, and only when the object's + # OWN class provides the collection's ordinary include? — not a Hash, + # Array or Set SUBCLASS overriding it (a case-insensitive allowlist, a + # fuzzy Set, a registry matching some other way entirely). `case allowed; + # when Hash ...` matches with ===, which for a Class is is_a?, so a + # subclass would otherwise match its ancestor's branch and have its + # override silently discarded — read for its raw keys/elements instead, + # which can invert which values it actually accepts. It is left opaque + # instead, exactly like any other object whose include? is the point + # (see resolve_in!) and enumerating it may be expensive (a DB-backed + # registry). # # A Hash lists its KEYS, which is what Hash#include? asks about — the - # Rails enum idiom, `in: Post.statuses` — and, like a Set, stays a Set, - # so membership stays O(1) per request. An Enumerator (Lazy included) is - # forced to an Array here, once: left lazy, it would be cast on every - # request instead of at class load. + # Rails enum idiom, `in: Post.statuses` — and, like a Set, is stored as a + # Set, so membership stays O(1) per request. + # ActiveSupport::HashWithIndifferentAccess is the one Hash subclass + # accepted anyway: its include? override only canonicalises the argument + # (String/Symbol) before the SAME key lookup, so its keys are still + # exactly its members — and it is what a Rails enum's own reader + # (`Post.statuses`) actually returns. + # Enumerator::Lazy is the same story on the Enumerator side: Lazy + # overrides chain methods like map and select, but not include?, so it + # is still read as a list — and forced to an Array here, once, since + # left lazy it would be cast on every request instead of at class load. def in_list(allowed) case allowed - when Hash then allowed.keys.to_set - when Set then allowed - when Array, Enumerator then allowed.to_a + when Hash then allowed.keys.to_set if plain_hash?(allowed) + when Set then allowed if allowed.instance_of?(Set) + when Array then allowed.to_a if allowed.instance_of?(Array) + when Enumerator then allowed.to_a if allowed.method(:include?).owner == Enumerable end end + def plain_hash?(allowed) + allowed.instance_of?(Hash) || allowed.instance_of?(ActiveSupport::HashWithIndifferentAccess) + end + # A Symbol is read as its String: it is how Ruby spells a constant # string, and a request never carries one, so no cast accepts it as is. def cast_in_member(type, member) diff --git a/spec/json_schema_spec.rb b/spec/json_schema_spec.rb index abf5ded..2ed38f4 100644 --- a/spec/json_schema_spec.rb +++ b/spec/json_schema_spec.rb @@ -129,6 +129,26 @@ def include?(value) = %w[free pro].include?(value.to_s.downcase) expect(prop["x-permittable-custom-validation"]).to be(true) end + # A Hash/Array/Set subclass overriding include? is opaque exactly like + # the plain-Object and Enumerable allowlists above — its raw contents + # (keys, elements) are not what it actually matches, so no enum can + # honestly be published for it. + it "exports a Hash/Array/Set subclass overriding include? as custom validation too" do + # Non-empty: assert_satisfiable! reads any object's own empty? at + # class load, and this Hash subclass inherits Hash's — unrelated to + # its overridden include?, but a truly empty one would already fail + # that check on its own, before ever reaching the list/opaque split. + registry = Class.new(Hash) { def include?(value) = value.to_s.start_with?("custom-") }.new + registry[:unrelated] = 1 + allowlist = Class.new(Array) { def include?(value) = any? { |c| c.to_s.casecmp?(value.to_s) } }.new(%w[pro]) + fuzzy = Class.new(Set) { def include?(value) = any? { |c| c.to_s.include?(value.to_s) } }.new(%w[pro]) + [registry, allowlist, fuzzy].each do |allowed| + prop = property("sku") { optional :sku, :string, in: allowed } + expect(prop).not_to have_key("enum") + expect(prop["x-permittable-custom-validation"]).to be(true) + end + end + it "carries a non-numeric Range as an extension instead of guessing" do prop = property("code") { optional :code, :string, in: "a".."m" } expect(prop["x-permittable-range"]).to eq('"a".."m"') diff --git a/spec/matchers_spec.rb b/spec/matchers_spec.rb index 6e33fe4..6ac7fc7 100644 --- a/spec/matchers_spec.rb +++ b/spec/matchers_spec.rb @@ -87,15 +87,22 @@ def failure_of it "reads within's argument exactly as the contract reads in: — a Hash as its keys, an allowlist as itself" do allowlist = Object.new def allowlist.include?(_value) = true + registry = Class.new(Hash) { def include?(value) = value.to_s.start_with?("custom-") }.new + registry[:unrelated] = 1 contract = Permittable::Contract.define do optional :status, :string, in: { draft: 0, published: 1 } optional :sku, :string, in: allowlist optional :tier, :string, in: [nil, "pro"], nullable: true + optional :code, :string, in: registry end expect(contract).to permit_param(:status).within({ draft: 0, published: 1 }) expect(contract).to permit_param(:status).within(%w[draft published]) expect(contract).to permit_param(:sku).within(allowlist) expect(contract).to permit_param(:tier).within([nil, "pro"]) + # A Hash subclass overriding include? is opaque, so within compares it + # as given — not by casting its keys, which would silently accept the + # wrong values. + expect(contract).to permit_param(:code).within(registry) end it "checks required and optional" do diff --git a/spec/permittable_spec.rb b/spec/permittable_spec.rb index 7146990..371c54d 100644 --- a/spec/permittable_spec.rb +++ b/spec/permittable_spec.rb @@ -144,6 +144,61 @@ def include?(value) = %w[free pro].include?(value.to_s.downcase) expect(violations_for({ plan: "gold" }, &decl).details).to eq([{ param: "plan", code: "inclusion" }]) end + # A Hash/Array/Set SUBCLASS overriding include? is the same story as the + # Enumerable above, by CLASS rather than by module: `case allowed; when + # Hash ...` matches with ===, which for a Class is is_a? — so a subclass + # matched the branch for its ancestor and had its override silently + # discarded, reading its raw contents (keys, elements) instead and + # inverting which values it actually accepts. Each is kept exactly as + # given, like any other object whose include? is the point. + it "keeps a Hash subclass's own include?, not its keys, when the override differs from Hash's" do + registry = Class.new(Hash) do + def include?(value) = value.to_s.start_with?("custom-") + end.new + registry[:unrelated] = 1 + decl = proc { permit_params(:create) { optional :sku, :string, in: registry } } + expect(permittable_class(&decl).permit_rule_for(:create)[:fields].first[:in]).to be(registry) + expect(permit({ sku: "custom-1" }, &decl)[:sku]).to eq("custom-1") + expect(violations_for({ sku: "unrelated" }, &decl).details).to eq([{ param: "sku", code: "inclusion" }]) + end + + it "keeps an Array subclass's own include?, not its elements" do + allowlist = Class.new(Array) do + def include?(value) = any? { |candidate| candidate.to_s.casecmp?(value.to_s) } + end.new(%w[free pro]) + decl = proc { permit_params(:create) { optional :plan, :string, in: allowlist } } + expect(permittable_class(&decl).permit_rule_for(:create)[:fields].first[:in]).to be(allowlist) + expect(permit({ plan: "PRO" }, &decl)[:plan]).to eq("PRO") + expect(violations_for({ plan: "gold" }, &decl).details).to eq([{ param: "plan", code: "inclusion" }]) + end + + it "keeps a Set subclass's own include?, not its elements" do + fuzzy = Class.new(Set) do + def include?(value) = any? { |candidate| candidate.to_s.include?(value.to_s) } + end.new(%w[free pro]) + decl = proc { permit_params(:create) { optional :plan, :string, in: fuzzy } } + expect(permittable_class(&decl).permit_rule_for(:create)[:fields].first[:in]).to be(fuzzy) + expect(permit({ plan: "p" }, &decl)[:plan]).to eq("p") + expect(violations_for({ plan: "gold" }, &decl).details).to eq([{ param: "plan", code: "inclusion" }]) + end + + # A plain Hash's keys are still cast to a Set for O(1) membership, and + # HashWithIndifferentAccess — a Hash SUBCLASS — is the one deliberate + # exception to the rule above: its include? override only canonicalises + # the argument (String/Symbol) before the same key lookup, so its keys + # are still exactly its members. It is what a Rails enum's own reader + # (`Post.statuses`) actually returns. + it "still reads a plain Hash and a HashWithIndifferentAccess as their keys" do + decl = proc do + permit_params(:create) do + optional :status, :string, in: { draft: 0, published: 1 } + optional :tier, :string, in: ActiveSupport::HashWithIndifferentAccess.new(free: "f", pro: "p") + end + end + fields = permittable_class(&decl).permit_rule_for(:create)[:fields] + expect(fields.map { |f| f[:in] }).to eq([Set["draft", "published"], Set["free", "pro"]]) + end + # The approved snapshot: a list is cast once, so a later `PLANS << "gold"` # is not seen. An app that needs a live list passes its own include? # object, which is read on every request.